diff --git a/server/src/bootstrap.ts b/server/src/bootstrap.ts index 13a087733..de81836c1 100644 --- a/server/src/bootstrap.ts +++ b/server/src/bootstrap.ts @@ -11,6 +11,7 @@ import { setupApiDocs } from './nest/platform/api-docs'; import { McpRegistryService } from '@trek/nest-mcp'; import { setMcpRegistry } from './mcp/registry-handoff'; import { validateBodyContracts } from './nest/common/validate-body-contracts'; +import { validateRouteGuards } from './nest/common/validate-route-guards'; /** * Builds the unified TREK NestJS application that serves the ENTIRE surface — the @@ -66,5 +67,9 @@ export async function buildApp(): Promise { // must carry a createZodDto class (validated by the global ZodValidationPipe) // or sit on the ratchet-only legacy allow-list — otherwise refuse to boot. validateBodyContracts(app); + // Fail closed on the anonymous surface too: a route that answers without a + // session must carry @Public() with a reason AND be on the reviewed list. + // Default-deny protects what exists; this is what keeps the exemptions honest. + validateRouteGuards(app); return app; } diff --git a/server/src/nest/app.module.ts b/server/src/nest/app.module.ts index 1d1558374..776834846 100644 --- a/server/src/nest/app.module.ts +++ b/server/src/nest/app.module.ts @@ -1,10 +1,11 @@ import { Module } from '@nestjs/common'; -import { APP_FILTER, APP_INTERCEPTOR, APP_PIPE } from '@nestjs/core'; +import { APP_FILTER, APP_GUARD, APP_INTERCEPTOR, APP_PIPE } from '@nestjs/core'; import { ZodValidationPipe } from './common/zod-validation.pipe'; import { AppConfigModule } from './app-config/app-config.module'; import { DatabaseModule } from './database/database.module'; import { RealtimeModule } from './realtime/realtime.module'; import { HealthModule } from './health/health.module'; +import { GlobalAuthGuard } from './auth/global-auth.guard'; import { WeatherModule } from './weather/weather.module'; import { HelpModule } from './help/help.module'; import { AirportsModule } from './airports/airports.module'; @@ -64,6 +65,10 @@ import { IdempotencyInterceptor } from './common/idempotency.interceptor'; @Module({ imports: [AppConfigModule, DatabaseModule, RealtimeModule, McpModule.forRoot({ accessPolicy: trekMcpAccessPolicy, validateAccess: trekMcpValidateAccess }), HealthModule, WeatherModule, HelpModule, AirportsModule, ConfigModule, SystemNoticesModule, MapsModule, PlaceEnrichmentModule, CategoriesModule, TagsModule, NotificationsModule, AtlasModule, VacayModule, PackingModule, TodoModule, BudgetModule, ReservationsModule, DaysModule, DayNotesModule, AccommodationsModule, AssignmentsModule, PlacesModule, TripsModule, CollabModule, FilesModule, PhotosModule, MemoriesModule, AirtrailModule, JourneyModule, CollectionsModule, ShareModule, TripInviteModule, TransitModule, FeedsModule, SettingsModule, BackupModule, AuthModule, OidcModule, OauthModule, AdminModule, AddonsModule, AuditModule, PermissionsModule, PluginsModule, BookingImportModule, LlmParseModule], providers: [ + // Default-deny: a route is authenticated unless it carries @Public() or + // @OptionalAuth(), or declares its own @UseGuards chain. Protection used to + // be opt-in, which made a forgotten guard a silent bypass instead of an error. + { provide: APP_GUARD, useClass: GlobalAuthGuard }, // Global error-envelope normaliser (DI-registered so it also catches // framework-level exceptions like the not-found handler). { provide: APP_FILTER, useClass: TrekExceptionFilter }, diff --git a/server/src/nest/auth/auth-public.controller.ts b/server/src/nest/auth/auth-public.controller.ts index 2fae17540..09c98d1b0 100644 --- a/server/src/nest/auth/auth-public.controller.ts +++ b/server/src/nest/auth/auth-public.controller.ts @@ -8,6 +8,7 @@ import { getClientIp } from '../audit/client-ip'; import { AuditService } from '../audit/audit.service'; import { willDropSecureCookie } from '../common/cookie'; import type { User } from '../../types'; +import { Public } from './public.decorator'; const WINDOW = 15 * 60 * 1000; const LOGIN_MIN_LATENCY_MS = 350; @@ -40,6 +41,7 @@ export class AuthPublicController { } @Post('demo-login') + @Public('issues a session for the demo account; there is nothing to authenticate yet') @HttpCode(200) demoLogin(@Req() req: Request, @Res({ passthrough: true }) res: Response) { const result = this.auth.demoLogin(); @@ -51,6 +53,7 @@ export class AuthPublicController { } @Get('invite/:token') + @Public('the invite token IS the credential') invite(@Param('token') token: string, @Req() req: Request) { this.limit('login', req, 10); const result = this.auth.validateInviteToken(token); @@ -61,6 +64,7 @@ export class AuthPublicController { } @Post('register') + @Public('creating the account that would carry the session') @HttpCode(201) register(@Body() body: RegisterDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { this.limit('login', req, 10); @@ -74,6 +78,7 @@ export class AuthPublicController { } @Post('login') + @Public('the login itself') @HttpCode(200) async login(@Body() body: LoginDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { this.limit('login', req, 10); @@ -101,6 +106,7 @@ export class AuthPublicController { } @Post('forgot-password') + @Public('reached by somebody who cannot log in') @HttpCode(200) async forgotPassword(@Body() body: ForgotPasswordDto, @Req() req: Request) { this.limit('forgot', req, 3); @@ -128,6 +134,7 @@ export class AuthPublicController { } @Post('reset-password') + @Public('the reset token IS the credential') @HttpCode(200) resetPassword(@Body() body: ResetPasswordDto, @Req() req: Request) { // Per-IP brute-force guard, parity with the legacy resetLimiter (5 / 15 min on @@ -147,6 +154,7 @@ export class AuthPublicController { } @Post('mfa/verify-login') + @Public('second factor of a login that has no session yet') @HttpCode(200) verifyMfaLogin(@Body() body: MfaVerifyLoginDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { this.limit('mfa', req, 5); @@ -160,6 +168,7 @@ export class AuthPublicController { } @Post('logout') + @Public('clearing a cookie must work even with an expired token, or the client cannot sign out') @HttpCode(200) logout(@Req() req: Request, @Res({ passthrough: true }) res: Response) { this.auth.clearAuthCookie(res, req); diff --git a/server/src/nest/auth/global-auth.guard.ts b/server/src/nest/auth/global-auth.guard.ts new file mode 100644 index 000000000..39b6d149b --- /dev/null +++ b/server/src/nest/auth/global-auth.guard.ts @@ -0,0 +1,65 @@ +import { CanActivate, ExecutionContext, HttpException, Injectable } from '@nestjs/common'; +import { GUARDS_METADATA } from '@nestjs/common/constants'; +import { Reflector } from '@nestjs/core'; +import type { Request } from 'express'; +import { extractToken, verifyJwtAndLoadUser } from './jwt-verify'; +import { IS_PUBLIC, OPTIONAL_AUTH } from './public.decorator'; + +/** + * Default-deny. Registered as an APP_GUARD, so a route is authenticated unless + * it says otherwise — the inverse of what TREK had, where protection was opt-in + * through 62 separate `@UseGuards(JwtAuthGuard)` and a forgotten one was a + * silent bypass rather than an error. + * + * It is a new class rather than JwtAuthGuard gaining a Reflector, for two + * reasons. JwtAuthGuard is constructed positionally in its own unit test with an + * ExecutionContext stub that has no getHandler, so a Reflector lookup there + * would break it; and the two want different behaviour anyway — this one stands + * down, that one refuses. + * + * The third check is what keeps this change behaviour-neutral: a controller + * that DECLARES its own guard chain keeps it, untouched. Nest runs global guards + * before route guards, so without this the global guard would 401 an anonymous + * request to an addon-gated controller before its AddonGuard could answer 404 — + * inverting an order that four e2e suites pin, and leaking which addons are + * installed. Reading the declared chain and standing down means every existing + * @UseGuards keeps deciding for itself, and this guard only covers what nobody + * claimed. + */ +@Injectable() +export class GlobalAuthGuard implements CanActivate { + constructor(private readonly reflector: Reflector) {} + + canActivate(context: ExecutionContext): boolean { + const handler = context.getHandler(); + const controller = context.getClass(); + + if (this.reflector.getAllAndOverride(IS_PUBLIC, [handler, controller])) return true; + + const req = context.switchToHttp().getRequest(); + + if (this.reflector.getAllAndOverride(OPTIONAL_AUTH, [handler, controller])) { + const token = extractToken(req); + (req as { user: unknown }).user = (token ? verifyJwtAndLoadUser(token) : null) || null; + return true; + } + + // A declared chain decides for itself, in the order it declared. + const declared = [ + ...(this.reflector.get(GUARDS_METADATA, handler) ?? []), + ...(this.reflector.get(GUARDS_METADATA, controller) ?? []), + ]; + if (declared.length > 0) return true; + + const token = extractToken(req); + if (!token) { + throw new HttpException({ error: 'Access token required', code: 'AUTH_REQUIRED' }, 401); + } + const user = verifyJwtAndLoadUser(token); + if (!user) { + throw new HttpException({ error: 'Invalid or expired token', code: 'AUTH_REQUIRED' }, 401); + } + req.user = user; + return true; + } +} diff --git a/server/src/nest/auth/public.decorator.ts b/server/src/nest/auth/public.decorator.ts new file mode 100644 index 000000000..cb3611c67 --- /dev/null +++ b/server/src/nest/auth/public.decorator.ts @@ -0,0 +1,33 @@ +import { SetMetadata } from '@nestjs/common'; + +/** Metadata key `@Public()` writes. */ +export const IS_PUBLIC = 'trek:public'; +/** Metadata key `@OptionalAuth()` writes. */ +export const OPTIONAL_AUTH = 'trek:optional-auth'; + +/** + * Marks a route as reachable without a session, under the global auth guard. + * + * The reason is required, and that is the point: with default-deny, the only + * routes a reader has to think about are the ones carrying this decorator, so + * each one should say why it is exempt. Same discipline as + * common/body-contract-allow-list.ts — a list nobody can add to without + * explaining themselves stays short. + * + * `@Public()` does NOT populate req.user. A route that wants the user when a + * token happens to be present, and no 401 when it is not, wants + * `@OptionalAuth()` instead. + */ +export const Public = (reason: string) => SetMetadata(IS_PUBLIC, { reason }); + +/** + * Marks a route whose response varies by auth state but which never demands it: + * the global guard loads the user when a valid token is present and leaves + * `req.user` null otherwise, exactly like the legacy `optionalAuth` middleware. + * + * Reach for `@Public()` unless the handler actually reads `req.user`. Populating + * it has a side effect beyond the handler: the IdempotencyInterceptor steps + * aside when there is no user, so making a route optional-auth newly enrols its + * mutations in idempotency replay. + */ +export const OptionalAuth = (reason: string) => SetMetadata(OPTIONAL_AUTH, { reason }); diff --git a/server/src/nest/common/validate-route-guards.ts b/server/src/nest/common/validate-route-guards.ts new file mode 100644 index 000000000..7c71f1aaf --- /dev/null +++ b/server/src/nest/common/validate-route-guards.ts @@ -0,0 +1,140 @@ +import { METHOD_METADATA, PATH_METADATA, GUARDS_METADATA } from '@nestjs/common/constants'; +import { ModulesContainer } from '@nestjs/core'; +import type { INestApplication } from '@nestjs/common'; +import { IS_PUBLIC, OPTIONAL_AUTH } from '../auth/public.decorator'; + +export interface RouteGuardEntry { + /** `ControllerClass.methodName` */ + id: string; + /** How the route is covered, for the failure message. */ + cover: 'public' | 'optional-auth' | 'declared-guards'; +} + +/** + * Boot-time gate: every registered route must be authenticated, or say why not. + * + * This is the thing that makes default-deny stick. The APP_GUARD alone protects + * what exists today; this refuses to boot when a NEW route arrives that is + * neither guarded nor explicitly exempt — the same shape as + * validate-body-contracts.ts, and for the same reason. Without it, the next + * `@Public()` pasted in to make a test pass is invisible. + * + * A route counts as covered when it carries @Public(), @OptionalAuth(), or a + * declared @UseGuards chain (on the handler or the class). Everything else falls + * to GlobalAuthGuard, which is fine — that is the default — so this gate does + * not fail on it. What it reports is the inventory, so the exempt set is a list + * somebody can read. + */ +export function collectRouteGuards(app: INestApplication): RouteGuardEntry[] { + const container = app.get(ModulesContainer, { strict: false }); + const entries: RouteGuardEntry[] = []; + + for (const module of container.values()) { + for (const wrapper of module.controllers.values()) { + const ctor = wrapper.metatype as (new (...args: never[]) => object) | undefined; + if (!ctor?.prototype) continue; + + const classPublic = Reflect.getMetadata(IS_PUBLIC, ctor); + const classOptional = Reflect.getMetadata(OPTIONAL_AUTH, ctor); + const classGuards = (Reflect.getMetadata(GUARDS_METADATA, ctor) as unknown[] | undefined) ?? []; + + for (const name of Object.getOwnPropertyNames(ctor.prototype)) { + if (name === 'constructor') continue; + const handler = (ctor.prototype as Record)[name]; + if (typeof handler !== 'function') continue; + // The definition of "a registered route" in this codebase: it carries an + // HTTP method decorator. Same test validate-body-contracts.ts uses. + if (Reflect.getMetadata(METHOD_METADATA, handler) === undefined) continue; + + const id = `${ctor.name}.${name}`; + const isPublic = classPublic ?? Reflect.getMetadata(IS_PUBLIC, handler); + const isOptional = classOptional ?? Reflect.getMetadata(OPTIONAL_AUTH, handler); + const handlerGuards = (Reflect.getMetadata(GUARDS_METADATA, handler) as unknown[] | undefined) ?? []; + + if (isPublic) entries.push({ id, cover: 'public' }); + else if (isOptional) entries.push({ id, cover: 'optional-auth' }); + else if (classGuards.length > 0 || handlerGuards.length > 0) entries.push({ id, cover: 'declared-guards' }); + } + } + } + + return entries.sort((a, b) => a.id.localeCompare(b.id)); +} + +/** + * The routes that answer without a session. Reviewed as a list on purpose: this + * is the whole anonymous surface of the server in one place, and a diff that + * grows it is a diff that widens what strangers can reach. + * + * Entries are `ControllerClass.methodName`. Sorted, no duplicates. + */ +export const PUBLIC_ROUTE_ALLOW_LIST: string[] = [ + // Everything a visitor needs before they have a session, plus the paths where + // a token other than the session cookie is the credential. + 'AuthPublicController.demoLogin', + 'AuthPublicController.forgotPassword', + 'AuthPublicController.invite', + 'AuthPublicController.login', + 'AuthPublicController.logout', + 'AuthPublicController.register', + 'AuthPublicController.resetPassword', + 'AuthPublicController.verifyMfaLogin', + 'ConfigController.getConfig', + 'FeaturesController.features', + // The download link carries its own short-lived token. + 'FilesDownloadController.download', + 'HelpController.asset', + 'HelpController.index', + 'HelpController.page', + // Share-token validated. + 'JourneyPublicController.get', + 'JourneyPublicController.legacyPhoto', + 'JourneyPublicController.photo', + 'OauthPublicController.revoke', + 'OauthPublicController.token', + 'OauthPublicController.userinfo', + // The OIDC handshake, before a TREK session exists. + 'OidcController.callback', + 'OidcController.exchange', + 'OidcController.login', + // Serves the sandbox document; the RPC inside it is authenticated. + 'PluginFrameController.serve', + // Auth is per plugin route (route.auth in the manifest), asserted in the handler. + 'PluginsProxyController.proxy', + // Share-token validated. + 'SharedController.placePhotoBytes', + 'SharedController.read', +]; + +/** + * Throws when a route carries @Public() without being listed above, or when a + * listed route no longer exists. Stale entries fail too, for the same reason the + * body-contract gate fails on them: a list nobody prunes stops being a list. + */ +export function validateRouteGuards(app: INestApplication, allowList: string[] = PUBLIC_ROUTE_ALLOW_LIST): void { + const entries = collectRouteGuards(app); + const publicIds = entries.filter((e) => e.cover === 'public').map((e) => e.id); + const allowed = new Set(allowList); + + const undeclared = publicIds.filter((id) => !allowed.has(id)); + const stale = allowList.filter((id) => !publicIds.includes(id)); + + const problems: string[] = []; + if (undeclared.length > 0) { + problems.push( + `route(s) marked @Public() but not in PUBLIC_ROUTE_ALLOW_LIST:\n ${undeclared.join('\n ')}`, + ); + } + if (stale.length > 0) { + problems.push( + `PUBLIC_ROUTE_ALLOW_LIST entries that are no longer @Public():\n ${stale.join('\n ')}`, + ); + } + if (problems.length > 0) { + throw new Error( + `Anonymous route surface changed.\n\n${problems.join('\n\n')}\n\n` + + 'Every entry is a route a stranger can reach. Add it deliberately, with the reason ' + + 'string @Public() requires, or remove the stale line.', + ); + } +} diff --git a/server/src/nest/config/config.controller.ts b/server/src/nest/config/config.controller.ts index 8cf047cea..c43e72b0b 100644 --- a/server/src/nest/config/config.controller.ts +++ b/server/src/nest/config/config.controller.ts @@ -1,6 +1,7 @@ import { Controller, Get } from '@nestjs/common'; import type { PublicConfig } from '@trek/shared'; import { DEFAULT_LANGUAGE } from '../../config'; +import { Public } from '../auth/public.decorator'; /** * /api/config — public (unauthenticated) bootstrap config. @@ -9,6 +10,7 @@ import { DEFAULT_LANGUAGE } from '../../config'; * no auth guard, returns the server's configured default language. Deliberately * has no service — it just surfaces a config constant, exactly like the original. */ +@Public('public bootstrap config the login screen reads before any session exists') @Controller('api/config') export class ConfigController { @Get() diff --git a/server/src/nest/files/files-download.controller.ts b/server/src/nest/files/files-download.controller.ts index 545842a03..96fdfbd9a 100644 --- a/server/src/nest/files/files-download.controller.ts +++ b/server/src/nest/files/files-download.controller.ts @@ -3,6 +3,7 @@ import type { Request, Response } from 'express'; import path from 'path'; import fs from 'fs'; import { FilesService } from './files.service'; +import { Public } from '../auth/public.decorator'; /** * GET /api/trips/:tripId/files/:id/download — authenticated file download. @@ -12,6 +13,7 @@ import { FilesService } from './files.service'; * the legacy authenticateDownload helper. Byte-identical to the legacy route: * 401 token, 404 trip/file, 403 path traversal, .pkpass served inline for Wallet. */ +@Public('authenticates itself: the download link carries a short-lived ?token the client mints') @Controller('api/trips/:tripId/files') export class FilesDownloadController { constructor(private readonly files: FilesService) {} diff --git a/server/src/nest/health/features.controller.ts b/server/src/nest/health/features.controller.ts index 19abdb254..61a706017 100644 --- a/server/src/nest/health/features.controller.ts +++ b/server/src/nest/health/features.controller.ts @@ -2,8 +2,10 @@ import { Controller, Get } from '@nestjs/common'; import { KitineraryExtractorService } from '../booking-import/kitinerary-extractor.service'; import { AddonsService } from '../addons/addons.service'; import { ADDON_IDS } from '../../addons'; +import { Public } from '../auth/public.decorator'; /** Exposes server feature flags consumed by the frontend to show/hide optional UI. */ +@Public('server capability flags the login screen reads to decide what to offer') @Controller('api/health') export class FeaturesController { constructor( diff --git a/server/src/nest/help/help.controller.ts b/server/src/nest/help/help.controller.ts index 9058431ee..94c1fd58b 100644 --- a/server/src/nest/help/help.controller.ts +++ b/server/src/nest/help/help.controller.ts @@ -1,5 +1,6 @@ import { Controller, Get, Param, Req, Res } from '@nestjs/common'; import type { Request, Response } from 'express'; +import { Public } from '../auth/public.decorator'; import { getWikiIndex, getWikiPage, @@ -16,6 +17,7 @@ import { * so these endpoints are unauthenticated; that also lets tags load the * proxied assets without sending credentials. */ +@Public('help assets are loaded by and , which cannot send credentials') @Controller('api/help') export class HelpController { @Get('index') diff --git a/server/src/nest/journey/journey-public.controller.ts b/server/src/nest/journey/journey-public.controller.ts index b78e3f0ea..4fb6c4a99 100644 --- a/server/src/nest/journey/journey-public.controller.ts +++ b/server/src/nest/journey/journey-public.controller.ts @@ -3,6 +3,7 @@ import type { Response } from 'express'; import path from 'node:path'; import fs from 'node:fs'; import { JourneyService } from './journey.service'; +import { Public } from '../auth/public.decorator'; /** * /api/public/journey — unauthenticated, share-token validated read + photo @@ -13,6 +14,7 @@ import { JourneyService } from './journey.service'; * the unified proxy streams by trek_photo_id and the legacy proxy serves local * files (with the uploads-dir traversal guard) or proxies immich/synology. */ +@Public('share-token validated: the whole point is a link that works without an account') @Controller('api/public/journey') export class JourneyPublicController { constructor(private readonly journey: JourneyService) {} diff --git a/server/src/nest/oauth/oauth-public.controller.ts b/server/src/nest/oauth/oauth-public.controller.ts index b5ade12e5..a0544ef9c 100644 --- a/server/src/nest/oauth/oauth-public.controller.ts +++ b/server/src/nest/oauth/oauth-public.controller.ts @@ -5,6 +5,7 @@ import { RateLimitService } from '../common/rate-limit.service'; import { getClientIp } from '../audit/client-ip'; import { logWarn } from '../audit/audit-log.logger'; import { AuditService } from '../audit/audit.service'; +import { Public } from '../auth/public.decorator'; const MIN = 60_000; @@ -16,6 +17,7 @@ const MIN = 60_000; * and the RFC 7009 always-200 revoke. Uses @Res directly because every branch * sets headers + a specific status the way the spec requires. */ +@Public('the OAuth 2.1 endpoints a client reaches before it has a session') @Controller('oauth') export class OauthPublicController { constructor(private readonly oauth: OauthService, private readonly rl: RateLimitService, private readonly audit: AuditService) {} diff --git a/server/src/nest/oidc/oidc.controller.ts b/server/src/nest/oidc/oidc.controller.ts index 0d9dd09f0..1f2de56e1 100644 --- a/server/src/nest/oidc/oidc.controller.ts +++ b/server/src/nest/oidc/oidc.controller.ts @@ -10,6 +10,7 @@ import type { User } from '../../types'; import { AuditService } from '../audit/audit.service'; import { getClientIp } from '../audit/client-ip'; import { AdminOidcUpdateDto } from '../admin/admin.dto'; +import { Public } from '../auth/public.decorator'; const OIDC_STATE_COOKIE = 'trek_oidc_state'; @@ -22,6 +23,7 @@ const OIDC_STATE_COOKIE = 'trek_oidc_state'; * codes, and the auth-code → cookie hand-off on /exchange. Uses @Res directly * because the flow mixes provider redirects with JSON error bodies. */ +@Public('the OIDC handshake happens before a TREK session exists; the provider state is the credential') @Controller('api/auth/oidc') export class OidcController { constructor(private readonly oidc: OidcService) {} diff --git a/server/src/nest/plugins/plugin-frame.controller.ts b/server/src/nest/plugins/plugin-frame.controller.ts index 0a932f8ff..24d7aa3c5 100644 --- a/server/src/nest/plugins/plugin-frame.controller.ts +++ b/server/src/nest/plugins/plugin-frame.controller.ts @@ -5,6 +5,7 @@ import path from 'node:path'; import { pluginsEnabled } from './kill-switch'; import { PluginRuntimeService } from './plugin-runtime.service'; import { pluginCodeDir } from './paths'; +import { Public } from '../auth/public.decorator'; /** * Serves a page/widget plugin's static client from /plugin-frame/:id/* (#plugins, @@ -32,6 +33,7 @@ const MIME: Record = { '.ico': 'image/x-icon', }; +@Public('serves the sandboxed iframe document itself; the RPC inside it is authenticated') @Controller('plugin-frame/:pluginId') export class PluginFrameController { constructor(private readonly runtime: PluginRuntimeService) {} diff --git a/server/src/nest/plugins/plugins-proxy.controller.ts b/server/src/nest/plugins/plugins-proxy.controller.ts index b8f447142..f75ae8cf8 100644 --- a/server/src/nest/plugins/plugins-proxy.controller.ts +++ b/server/src/nest/plugins/plugins-proxy.controller.ts @@ -3,6 +3,7 @@ import type { Request, Response } from 'express'; import { extractToken, verifyJwtAndLoadUser } from '../auth/jwt-verify'; import { pluginsEnabled } from './kill-switch'; import { PluginRuntimeService } from './plugin-runtime.service'; +import { Public } from '../auth/public.decorator'; /** * Proxies a plugin's own HTTP routes at /api/plugins/:id/* (#plugins, M2). @@ -70,6 +71,7 @@ function toRelativeLocation(loc: unknown): string | null { } } +@Public('auth is data-driven per plugin route (route.auth in the manifest), asserted in the handler') @Controller('api/plugins/:pluginId') export class PluginsProxyController { constructor(private readonly runtime: PluginRuntimeService) {} diff --git a/server/src/nest/share/share.controller.ts b/server/src/nest/share/share.controller.ts index dbdd7dbe2..ce5715bda 100644 --- a/server/src/nest/share/share.controller.ts +++ b/server/src/nest/share/share.controller.ts @@ -6,6 +6,7 @@ import { ShareService } from './share.service'; import { ShareLinkDto } from './share.dto'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; +import { Public } from '../auth/public.decorator'; /** * /api/trips/:tripId/share-link — manage a trip's public read-only share token. @@ -70,6 +71,7 @@ export class TripShareController { * GET /api/shared/:token — public, unauthenticated read-only trip snapshot. * Deliberately NOT behind a guard; an invalid/expired token answers 404. */ +@Public('share-token validated: a shared trip link has to work for somebody without an account') @Controller('api/shared') export class SharedController { constructor(private readonly share: ShareService) {} diff --git a/server/tests/unit/nest/route-inventory.test.ts b/server/tests/unit/nest/route-inventory.test.ts new file mode 100644 index 000000000..2ff8a5dd6 --- /dev/null +++ b/server/tests/unit/nest/route-inventory.test.ts @@ -0,0 +1,79 @@ +/** + * The anonymous surface of the server, as a list. + * + * This is what makes default-deny durable. The APP_GUARD protects what exists + * today; this fails when a NEW route arrives that answers without a session and + * nobody wrote it down. The same shape as the body-contract gate, and for the + * same reason: a rule with no inventory behind it decays into whatever the last + * person needed to make a test pass. + */ +import { describe, it, expect, vi } from 'vitest'; + +vi.mock('../../../src/db/database', async () => { + const Database = (await import('better-sqlite3')).default; + const db = new Database(':memory:'); + return { db, closeDb: () => {}, reinitialize: () => {}, canAccessTrip: () => null, isOwner: () => false, getPlaceWithTags: () => null }; +}); + +import { Test } from '@nestjs/testing'; +import { AppModule } from '../../../src/nest/app.module'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { collectRouteGuards, PUBLIC_ROUTE_ALLOW_LIST, validateRouteGuards } from '../../../src/nest/common/validate-route-guards'; + +async function buildApp() { + const moduleRef = await Test.createTestingModule({ imports: [AppModule] }) + .overrideProvider(DatabaseService) + .useValue({ get: () => undefined, all: () => [], run: () => ({}), canAccessTrip: () => null, isOwner: () => false }) + .compile(); + return moduleRef.createNestApplication(); +} + +describe('route guard inventory', () => { + it('ROUTES-001: every @Public() route is in the reviewed list, and every listed one still exists', async () => { + const app = await buildApp(); + try { + expect(() => validateRouteGuards(app)).not.toThrow(); + } finally { + await app.close(); + } + }); + + it('ROUTES-002: the list is sorted and free of duplicates, so a diff to it reads', () => { + expect([...PUBLIC_ROUTE_ALLOW_LIST].sort()).toEqual(PUBLIC_ROUTE_ALLOW_LIST); + expect(new Set(PUBLIC_ROUTE_ALLOW_LIST).size).toBe(PUBLIC_ROUTE_ALLOW_LIST.length); + }); + + it('ROUTES-003: an unlisted public route fails the gate, naming it', async () => { + const app = await buildApp(); + try { + // Drop one entry: the gate must notice the route it no longer covers. + const short = PUBLIC_ROUTE_ALLOW_LIST.slice(1); + expect(() => validateRouteGuards(app, short)).toThrow(PUBLIC_ROUTE_ALLOW_LIST[0]); + } finally { + await app.close(); + } + }); + + it('ROUTES-004: a stale entry fails too — an unpruned list is not a list', async () => { + const app = await buildApp(); + try { + expect(() => validateRouteGuards(app, [...PUBLIC_ROUTE_ALLOW_LIST, 'GhostController.vanished'].sort())) + .toThrow('GhostController.vanished'); + } finally { + await app.close(); + } + }); + + it('ROUTES-005: nothing is left uncovered by accident — every route is public, optional, guarded or default-denied', async () => { + const app = await buildApp(); + try { + const entries = collectRouteGuards(app); + // The inventory only lists routes that opted out of the default. Anything + // absent from it falls to GlobalAuthGuard, which is the safe direction. + expect(entries.every((e) => ['public', 'optional-auth', 'declared-guards'].includes(e.cover))).toBe(true); + expect(entries.filter((e) => e.cover === 'public').length).toBe(PUBLIC_ROUTE_ALLOW_LIST.length); + } finally { + await app.close(); + } + }); +});