diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts index 49914817d..7d04744d9 100644 --- a/server/src/nest/common/body-contract-allow-list.ts +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -73,9 +73,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ 'JourneyController.uploadEntryPhotos', 'JourneyController.uploadGalleryVideo', 'LlmLocalController.pull', - 'MapsController.autocomplete', - 'MapsController.resolveUrl', - 'MapsController.search', 'NotificationsController.respond', 'NotificationsController.setPreferences', 'NotificationsController.testNtfy', diff --git a/server/src/nest/maps/maps.controller.ts b/server/src/nest/maps/maps.controller.ts index e4c2e2052..4ba8b1fe6 100644 --- a/server/src/nest/maps/maps.controller.ts +++ b/server/src/nest/maps/maps.controller.ts @@ -24,8 +24,7 @@ import type { User } from '../../types'; import { MapsService } from './maps.service'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; - -type LocationBias = { low: { lat: number; lng: number }; high: { lat: number; lng: number } }; +import { MapsSearchDto, MapsAutocompleteDto, MapsResolveUrlDto } from './maps.dto'; /** Maps a thrown service error to the same status + { error } body Express sent. */ function toHttpException(err: unknown, fallbackMessage: string, defaultStatus: number): HttpException { @@ -38,11 +37,20 @@ function toHttpException(err: unknown, fallbackMessage: string, defaultStatus: n * /api/maps — place search, autocomplete, details, photos, reverse geocoding and * Google-Maps-URL resolution. * - * Behaviour is byte-identical to the legacy Express route (server/src/routes/ - * maps.ts): same auth, same bespoke 400 validation messages, the same - * per-endpoint kill-switch short-circuits, the same error status/body mapping, - * and the same diagnostic logging. The SSRF guard lives in the underlying + * Behaviour matches the legacy Express route (server/src/routes/maps.ts): same + * auth, same per-endpoint kill-switch short-circuits, same error status/body + * mapping, same diagnostic logging, and the same bespoke 400s for non-body + * validation (reverse's query params). The SSRF guard lives in the underlying * service and is reused unchanged. + * + * Bodies are validated against the @trek/shared maps schemas via maps.dto.ts + * (global ZodValidationPipe). This replaced the legacy bespoke 400s ('Search + * query is required', 'Input is required', 'Input too long (max 200 chars)', + * 'URL is required', the two 'Invalid locationBias: …' messages) with the + * pipe's uniform { error: 'field: message; …' } envelope — and the pipe runs + * before the handler, so an invalid autocomplete body now 400s even while the + * autocomplete kill-switch is on (the legacy route answered the disabled + * envelope first). */ @Controller('api/maps') @UseGuards(JwtAuthGuard) @@ -53,19 +61,11 @@ export class MapsController { @HttpCode(200) // Express answers with res.json (200); Nest would otherwise default POST to 201. async search( @CurrentUser() user: User, - @Body('query') query: unknown, + @Body() body: MapsSearchDto, @Query('lang') lang?: string, - @Body('locationBias') locationBias?: { lat: number; lng: number; radius?: number }, ): Promise { - if (!query) { - throw new HttpException({ error: 'Search query is required' }, 400); - } - // Optional bias toward a coordinate (lat/lng[/radius]); improves foreign-region queries. - if (locationBias && !(Number.isFinite(locationBias.lat) && Number.isFinite(locationBias.lng))) { - throw new HttpException({ error: 'Invalid locationBias: lat and lng must be finite numbers' }, 400); - } try { - return await this.maps.search(user.id, query as string, lang, locationBias); + return await this.maps.search(user.id, body.query, lang, body.locationBias); } catch (err: unknown) { console.error('Maps search error:', err); throw toHttpException(err, 'Search error', 500); @@ -98,29 +98,13 @@ export class MapsController { @HttpCode(200) async autocomplete( @CurrentUser() user: User, - @Body('input') input: unknown, - @Body('lang') lang?: string, - @Body('locationBias') locationBias?: LocationBias, + @Body() body: MapsAutocompleteDto, ): Promise { if (this.maps.autocompleteDisabled()) { return { suggestions: [], source: 'disabled' }; } - if (!input || typeof input !== 'string') { - throw new HttpException({ error: 'Input is required' }, 400); - } - if (input.length > 200) { - throw new HttpException({ error: 'Input too long (max 200 chars)' }, 400); - } - if (locationBias) { - const { low, high } = locationBias; - if (!low || !high - || !Number.isFinite(low.lat) || !Number.isFinite(low.lng) - || !Number.isFinite(high.lat) || !Number.isFinite(high.lng)) { - throw new HttpException({ error: 'Invalid locationBias: low and high must have finite lat and lng' }, 400); - } - } try { - return await this.maps.autocomplete(user.id, input, lang, locationBias); + return await this.maps.autocomplete(user.id, body.input, body.lang, body.locationBias); } catch (err: unknown) { console.error('Maps autocomplete error:', err); throw toHttpException(err, 'Autocomplete error', 500); @@ -226,12 +210,9 @@ export class MapsController { @Post('resolve-url') @HttpCode(200) - async resolveUrl(@Body('url') url: unknown): Promise { - if (!url || typeof url !== 'string') { - throw new HttpException({ error: 'URL is required' }, 400); - } + async resolveUrl(@Body() body: MapsResolveUrlDto): Promise { try { - return await this.maps.resolveUrl(url); + return await this.maps.resolveUrl(body.url); } catch (err: unknown) { const message = err instanceof Error ? err.message : 'Failed to resolve URL'; console.error('[Maps] URL resolve error:', message); diff --git a/server/src/nest/maps/maps.dto.ts b/server/src/nest/maps/maps.dto.ts new file mode 100644 index 000000000..bcfc88bb3 --- /dev/null +++ b/server/src/nest/maps/maps.dto.ts @@ -0,0 +1,16 @@ +import { createZodDto } from 'nestjs-zod'; +import { + mapsSearchRequestSchema, + mapsAutocompleteRequestSchema, + mapsResolveUrlRequestSchema, +} from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared maps contracts. The + * global ZodValidationPipe (APP_PIPE in app.module.ts) validates any @Body() + * parameter typed with one of these classes by metatype — the Zod schemas in + * shared/ remain the single source of truth for the wire contract. + */ +export class MapsSearchDto extends createZodDto(mapsSearchRequestSchema) {} +export class MapsAutocompleteDto extends createZodDto(mapsAutocompleteRequestSchema) {} +export class MapsResolveUrlDto extends createZodDto(mapsResolveUrlRequestSchema) {} diff --git a/server/tests/unit/nest/maps.controller.test.ts b/server/tests/unit/nest/maps.controller.test.ts index c269ae2c0..b0cb1acf7 100644 --- a/server/tests/unit/nest/maps.controller.test.ts +++ b/server/tests/unit/nest/maps.controller.test.ts @@ -36,46 +36,36 @@ beforeEach(() => { }); describe('MapsController (parity with the legacy /api/maps route)', () => { + // Body validation (required query/input/url, locationBias shapes, input + // length) moved to the @trek/shared maps schemas enforced by the global + // ZodValidationPipe — the pipe's uniform envelope replaced the legacy + // bespoke 400 strings, and the 400-status contract is pinned by the + // integration/e2e suites. These unit cases cover the handler bodies only. describe('POST /search', () => { - it('400 when query is missing', async () => { - expect(await thrown(() => makeController({}).search(user, undefined))).toEqual({ - status: 400, body: { error: 'Search query is required' }, - }); - }); - it('returns the service result', async () => { const search = vi.fn().mockResolvedValue({ places: [], source: 'osm' }); - const res = await makeController({ search }).search(user, 'berlin', 'de'); + const res = await makeController({ search }).search(user, { query: 'berlin' }, 'de'); expect(res).toEqual({ places: [], source: 'osm' }); expect(search).toHaveBeenCalledWith(3, 'berlin', 'de', undefined); }); - it('400 on a malformed locationBias (non-finite lat/lng)', async () => { - const search = vi.fn(); - const bad = { lat: NaN, lng: 2 }; - expect(await thrown(() => makeController({ search }).search(user, 'x', 'de', bad))).toEqual({ - status: 400, body: { error: 'Invalid locationBias: lat and lng must be finite numbers' }, - }); - expect(search).not.toHaveBeenCalled(); - }); - it('forwards a valid locationBias to the service', async () => { const search = vi.fn().mockResolvedValue({ places: [], source: 'osm' }); const bias = { lat: 1, lng: 2, radius: 5000 }; - await makeController({ search }).search(user, 'x', 'de', bias); + await makeController({ search }).search(user, { query: 'x', locationBias: bias }, 'de'); expect(search).toHaveBeenCalledWith(3, 'x', 'de', bias); }); it('maps a service error to its status + message', async () => { const search = vi.fn().mockRejectedValue(withError(429, 'Rate limited')); - expect(await thrown(() => makeController({ search }).search(user, 'x'))).toEqual({ + expect(await thrown(() => makeController({ search }).search(user, { query: 'x' }))).toEqual({ status: 429, body: { error: 'Rate limited' }, }); }); it('defaults a non-Error rejection to 500 + the fallback message', async () => { const search = vi.fn().mockRejectedValue('boom'); - expect(await thrown(() => makeController({ search }).search(user, 'x'))).toEqual({ + expect(await thrown(() => makeController({ search }).search(user, { query: 'x' }))).toEqual({ status: 500, body: { error: 'Search error' }, }); }); @@ -116,51 +106,22 @@ describe('MapsController (parity with the legacy /api/maps route)', () => { describe('POST /autocomplete', () => { it('returns the disabled envelope when the kill-switch is off', async () => { const autocomplete = vi.fn(); - const res = await makeController({ autocompleteDisabled: () => true, autocomplete }).autocomplete(user, 'be'); + const res = await makeController({ autocompleteDisabled: () => true, autocomplete }).autocomplete(user, { input: 'be' }); expect(res).toEqual({ suggestions: [], source: 'disabled' }); expect(autocomplete).not.toHaveBeenCalled(); }); - it('400 when input is missing or not a string', async () => { - const c = makeController({ autocompleteDisabled: () => false }); - expect(await thrown(() => c.autocomplete(user, undefined))).toEqual({ status: 400, body: { error: 'Input is required' } }); - expect(await thrown(() => c.autocomplete(user, 123 as unknown as string))).toEqual({ status: 400, body: { error: 'Input is required' } }); - }); - - it('400 when input is too long', async () => { - const c = makeController({ autocompleteDisabled: () => false }); - expect(await thrown(() => c.autocomplete(user, 'x'.repeat(201)))).toEqual({ - status: 400, body: { error: 'Input too long (max 200 chars)' }, - }); - }); - - it('400 on a malformed locationBias', async () => { - const c = makeController({ autocompleteDisabled: () => false }); - const bad = { low: { lat: 1, lng: NaN }, high: { lat: 2, lng: 3 } }; - expect(await thrown(() => c.autocomplete(user, 'be', undefined, bad))).toEqual({ - status: 400, body: { error: 'Invalid locationBias: low and high must have finite lat and lng' }, - }); - }); - - it('400 when locationBias is missing the high corner', async () => { - const c = makeController({ autocompleteDisabled: () => false }); - const bad = { low: { lat: 1, lng: 2 } } as never; - expect(await thrown(() => c.autocomplete(user, 'be', undefined, bad))).toEqual({ - status: 400, body: { error: 'Invalid locationBias: low and high must have finite lat and lng' }, - }); - }); - it('delegates a valid request', async () => { const autocomplete = vi.fn().mockResolvedValue({ suggestions: [], source: 'osm' }); const bias = { low: { lat: 1, lng: 2 }, high: { lat: 3, lng: 4 } }; - await makeController({ autocompleteDisabled: () => false, autocomplete }).autocomplete(user, 'be', 'en', bias); + await makeController({ autocompleteDisabled: () => false, autocomplete }).autocomplete(user, { input: 'be', lang: 'en', locationBias: bias }); expect(autocomplete).toHaveBeenCalledWith(3, 'be', 'en', bias); }); it('maps a service error', async () => { const autocomplete = vi.fn().mockRejectedValue(withError(503, 'Upstream down')); const c = makeController({ autocompleteDisabled: () => false, autocomplete }); - expect(await thrown(() => c.autocomplete(user, 'be'))).toEqual({ + expect(await thrown(() => c.autocomplete(user, { input: 'be' }))).toEqual({ status: 503, body: { error: 'Upstream down' }, }); }); @@ -329,38 +290,28 @@ describe('MapsController (parity with the legacy /api/maps route)', () => { }); describe('POST /resolve-url', () => { - it('400 when url missing or not a string', async () => { - expect(await thrown(() => makeController({}).resolveUrl(undefined))).toEqual({ status: 400, body: { error: 'URL is required' } }); - }); - it('returns the resolved coordinates', async () => { const resolveUrl = vi.fn().mockResolvedValue({ lat: 1, lng: 2, name: null, address: null }); - expect(await makeController({ resolveUrl }).resolveUrl('https://maps.app.goo.gl/x')).toEqual({ lat: 1, lng: 2, name: null, address: null }); - }); - - it('400 when url is not a string', async () => { - expect(await thrown(() => makeController({}).resolveUrl(42 as unknown as string))).toEqual({ - status: 400, body: { error: 'URL is required' }, - }); + expect(await makeController({ resolveUrl }).resolveUrl({ url: 'https://maps.app.goo.gl/x' })).toEqual({ lat: 1, lng: 2, name: null, address: null }); }); it('maps a service error, defaulting to 400', async () => { const resolveUrl = vi.fn().mockRejectedValue(new Error('Failed to resolve URL')); - expect(await thrown(() => makeController({ resolveUrl }).resolveUrl('bad'))).toEqual({ + expect(await thrown(() => makeController({ resolveUrl }).resolveUrl({ url: 'bad' }))).toEqual({ status: 400, body: { error: 'Failed to resolve URL' }, }); }); it('honours an explicit status on the thrown error', async () => { const resolveUrl = vi.fn().mockRejectedValue(withError(422, 'Unsupported link')); - expect(await thrown(() => makeController({ resolveUrl }).resolveUrl('bad'))).toEqual({ + expect(await thrown(() => makeController({ resolveUrl }).resolveUrl({ url: 'bad' }))).toEqual({ status: 422, body: { error: 'Unsupported link' }, }); }); it('falls back to the default message when a non-Error is thrown', async () => { const resolveUrl = vi.fn().mockRejectedValue('nope'); - expect(await thrown(() => makeController({ resolveUrl }).resolveUrl('bad'))).toEqual({ + expect(await thrown(() => makeController({ resolveUrl }).resolveUrl({ url: 'bad' }))).toEqual({ status: 400, body: { error: 'Failed to resolve URL' }, }); }); diff --git a/shared/src/maps/maps.schema.spec.ts b/shared/src/maps/maps.schema.spec.ts index e59287aa9..bc97643b6 100644 --- a/shared/src/maps/maps.schema.spec.ts +++ b/shared/src/maps/maps.schema.spec.ts @@ -13,6 +13,21 @@ describe('mapsSearchRequestSchema', () => { expect(mapsSearchRequestSchema.safeParse({ query: '' }).success).toBe(false); expect(mapsSearchRequestSchema.safeParse({}).success).toBe(false); }); + + it('allows an optional circle locationBias with numeric lat/lng and optional radius', () => { + expect( + mapsSearchRequestSchema.safeParse({ query: 'berlin', locationBias: { lat: 52.5, lng: 13.4 } }).success, + ).toBe(true); + expect( + mapsSearchRequestSchema.safeParse({ query: 'berlin', locationBias: { lat: 52.5, lng: 13.4, radius: 50000 } }) + .success, + ).toBe(true); + // NaN arrives as null over JSON; either way a non-numeric lat must fail. + expect( + mapsSearchRequestSchema.safeParse({ query: 'berlin', locationBias: { lat: null, lng: 13.4 } }).success, + ).toBe(false); + expect(mapsSearchRequestSchema.safeParse({ query: 'berlin', locationBias: { lat: 52.5 } }).success).toBe(false); + }); }); describe('mapsAutocompleteRequestSchema', () => { diff --git a/shared/src/maps/maps.schema.ts b/shared/src/maps/maps.schema.ts index 9e3318d7b..5daf767ef 100644 --- a/shared/src/maps/maps.schema.ts +++ b/shared/src/maps/maps.schema.ts @@ -10,15 +10,22 @@ import { z } from 'zod'; * source, so the response schemas keep them as open records — the contract pins * down the request shapes and the stable envelope fields, not the provider blobs. * - * The bespoke 400 validation messages and the per-endpoint kill-switch responses - * are reproduced in the controller, not derived from these schemas, so the bodies - * stay byte-identical to Express. + * Since the maps body-contract ratchet, the request schemas below are enforced + * on the server via createZodDto wrappers (maps.dto.ts) and the global + * ZodValidationPipe — invalid bodies get the pipe's uniform + * { error: 'field: message; …' } envelope. The per-endpoint kill-switch + * responses and the non-body validation (query params, URL params) keep their + * bespoke bodies in the controller. */ const latLng = z.object({ lat: z.number(), lng: z.number() }); export const mapsSearchRequestSchema = z.object({ query: z.string().min(1), + // Optional bias toward a coordinate (lat/lng[/radius]); improves + // foreign-region queries. z.number() is finite-only (zod v4), matching the + // legacy Number.isFinite() check; radius was never validated beyond "number". + locationBias: latLng.extend({ radius: z.number().optional() }).optional(), }); export type MapsSearchRequest = z.infer;