diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index 088882147..f0c7fb10f 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -3332,7 +3332,7 @@ describe('DayPlanSidebar', () => { render() fireEvent.contextMenu(dragRow(screen.getByText('Louvre'))) await user.click(screen.getByRole('button', { name: 'Open Website' })) - expect(openSpy).toHaveBeenCalledWith('https://louvre.fr', '_blank') + expect(openSpy).toHaveBeenCalledWith('https://louvre.fr', '_blank', 'noopener,noreferrer') fireEvent.contextMenu(dragRow(screen.getByText('Louvre'))) await user.click(screen.getByRole('button', { name: 'Google Maps' })) diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 3845ce953..e120c0c06 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -4,6 +4,7 @@ declare global { interface Window { __dragData: DragDataPayload | null } } import React, { useState, useEffect, useLayoutEffect, useRef, useMemo, useCallback } from 'react' import { avatarSrc } from '../../utils/avatarSrc' +import { safeHttpUrl } from '../../utils/safeUrl' import { ChevronDown, ChevronRight, ChevronUp, Compass, Navigation, RotateCcw, ExternalLink, Clock, Pencil, GripVertical, Ticket, Plus, FileText, Trash2, Car, Lock, Hotel, Footprints, Route as RouteIcon, Bookmark, TramFront, Zap } from 'lucide-react' import { type PickedPlace } from './TransitSearchPanel' import { assignmentsApi, reservationsApi, daysApi } from '../../api/client' @@ -1871,7 +1872,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP ctxMenu.open(e, [ canEditDays && onEditPlace && { label: t('common.edit'), icon: Pencil, onClick: () => onEditPlace(place, assignment.id) }, canEditDays && onRemoveAssignment && { label: t('planner.removeFromDay'), icon: Trash2, onClick: () => onRemoveAssignment(day.id, assignment.id) }, - place.website && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(place.website, '_blank') }, + safeHttpUrl(place.website) && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer') }, ...navTargets.map(target => ({ label: target.label, icon: Navigation, onClick: () => openNavigationTarget(target) })), collectionsEnabled && { label: t('inspector.saveToCollection'), icon: Bookmark, onClick: () => useSaveToCollectionStore.getState().open(placeToSaveTarget(place)) }, { divider: true }, diff --git a/client/src/components/Planner/PlaceInspector.test.tsx b/client/src/components/Planner/PlaceInspector.test.tsx index 0dcf4cc0a..2288cb010 100644 --- a/client/src/components/Planner/PlaceInspector.test.tsx +++ b/client/src/components/Planner/PlaceInspector.test.tsx @@ -953,7 +953,7 @@ describe('PlaceInspector', () => { fireEvent.click(screen.getByText('Open Website').closest('button')!); expect(open).toHaveBeenCalledTimes(2); - expect(open.mock.calls[1]).toEqual(['https://example.org', '_blank']); + expect(open.mock.calls[1]).toEqual(['https://example.org', '_blank', 'noopener,noreferrer']); vi.unstubAllGlobals(); }); diff --git a/client/src/components/Planner/PlaceInspector.tsx b/client/src/components/Planner/PlaceInspector.tsx index a4ca8d53b..e89ab380f 100644 --- a/client/src/components/Planner/PlaceInspector.tsx +++ b/client/src/components/Planner/PlaceInspector.tsx @@ -1,5 +1,6 @@ import React, { useState, useEffect, useRef, useCallback, useMemo } from 'react' import { avatarSrc } from '../../utils/avatarSrc' +import { safeHttpUrl } from '../../utils/safeUrl' import { openFile } from '../../utils/fileDownload' import Markdown from 'react-markdown' import remarkGfm from 'remark-gfm' @@ -318,6 +319,10 @@ export default function PlaceInspector({ detailLng, googleDetails?.open_now, ) + // Allow-listed rather than passed straight through: window.open runs a + // javascript: URL in this origin, and the stored value predates the check the + // server does on the way in now. + const websiteUrl = safeHttpUrl(place.website) ?? safeHttpUrl(googleDetails?.website) // Prefer the place's stored ftid; if it has none yet, use the one just fetched from Google. const navigationTargets = getNavigationTargets( place ? { ...place, google_ftid: place.google_ftid || googleDetails?.google_ftid || null } : null, @@ -513,8 +518,8 @@ export default function PlaceInspector({ )} )} - {(place.website || googleDetails?.website) && ( - window.open(place.website || googleDetails?.website, '_blank')} variant="ghost" icon={} + {websiteUrl && ( + window.open(websiteUrl, '_blank', 'noopener,noreferrer')} variant="ghost" icon={} label={{t('inspector.website')}} /> )}
diff --git a/client/src/components/Planner/usePlacesSidebar.test.tsx b/client/src/components/Planner/usePlacesSidebar.test.tsx index 1d79a4da5..03918897b 100644 --- a/client/src/components/Planner/usePlacesSidebar.test.tsx +++ b/client/src/components/Planner/usePlacesSidebar.test.tsx @@ -643,7 +643,7 @@ describe('usePlacesSidebar context menu', () => { fireEvent.contextMenu(screen.getByTestId('row-3')); await user.click(screen.getByRole('button', { name: 'Open Website' })); - expect(open).toHaveBeenCalledWith('https://cafe.example', '_blank'); + expect(open).toHaveBeenCalledWith('https://cafe.example', '_blank', 'noopener,noreferrer'); fireEvent.contextMenu(screen.getByTestId('row-3')); await user.click(screen.getByRole('button', { name: 'Google Maps' })); diff --git a/client/src/components/Planner/usePlacesSidebar.ts b/client/src/components/Planner/usePlacesSidebar.ts index 40b34b4d2..aab094a62 100644 --- a/client/src/components/Planner/usePlacesSidebar.ts +++ b/client/src/components/Planner/usePlacesSidebar.ts @@ -14,6 +14,7 @@ import { useSaveToCollectionStore } from '../../store/saveToCollectionStore' import { placeToSaveTarget } from '../Collections/saveTarget' import type { Place, Category, Day, AssignmentsMap } from '../../types' import { getGoogleMapsUrlForPlace } from './placeGoogleMaps' +import { safeHttpUrl } from '../../utils/safeUrl' export interface PlacesSidebarProps { tripId: number @@ -276,7 +277,7 @@ export function usePlacesSidebar(props: PlacesSidebarProps) { ctxMenu.open(e, [ canEditPlaces && { label: t('common.edit'), icon: Pencil, onClick: () => props.onEditPlace(place) }, selDayId && { label: t('planner.addToDay'), icon: CalendarDays, onClick: () => props.onAssignToDay(place.id, selDayId) }, - place.website && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(place.website, '_blank') }, + safeHttpUrl(place.website) && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer') }, googleMapsUrl && { label: t('inspector.google'), icon: Navigation, onClick: () => window.open(googleMapsUrl, '_blank') }, collectionsEnabled && { label: t('inspector.saveToCollection'), icon: Bookmark, onClick: () => useSaveToCollectionStore.getState().open(placeToSaveTarget(place)) }, { divider: true }, diff --git a/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx b/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx index 544a70b3b..d7e0360e5 100644 --- a/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx @@ -17,6 +17,7 @@ import PlaceRating from '../../../../components/shared/StarRating' import TrackColorPicker from '../../../../components/shared/TrackColorPicker' import { resolveTrackColor, inheritedTrackColor } from '../../../../components/Map/trackColors' import { avatarSrc } from '../../../../utils/avatarSrc' +import { safeHttpUrl } from '../../../../utils/safeUrl' import { openFile } from '../../../../utils/fileDownload' import { getNavigationTargets, openNavigationTarget } from '../../../../components/Planner/placeNavigation' import { NavigationMenu } from '../../../../components/shared/NavigationMenu' @@ -527,9 +528,9 @@ export default function MPlaceSheet({ planner, shell }: MTripSheetsProps) { )} - {place.website && ( + {safeHttpUrl(place.website) && ( window.open(place.website!, '_blank', 'noopener,noreferrer')} + onClick={() => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer')} label={t('inspector.website')} > diff --git a/client/src/utils/safeUrl.test.ts b/client/src/utils/safeUrl.test.ts new file mode 100644 index 000000000..f908a67b1 --- /dev/null +++ b/client/src/utils/safeUrl.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from 'vitest' +import { safeHttpUrl } from './safeUrl' + +describe('safeHttpUrl', () => { + it('FE-UTIL-SAFEURL-001: passes an ordinary https homepage through', () => { + expect(safeHttpUrl('https://louvre.fr/en/visit')).toBe('https://louvre.fr/en/visit') + }) + + it('FE-UTIL-SAFEURL-002: passes plain http through — plenty of small venues still run it', () => { + expect(safeHttpUrl('http://pension-alpenblick.at')).toBe('http://pension-alpenblick.at') + }) + + it('FE-UTIL-SAFEURL-003: rejects javascript:, which window.open would run in this origin', () => { + expect(safeHttpUrl('javascript:fetch("/api/trips")')).toBeNull() + expect(safeHttpUrl('JavaScript:alert(1)')).toBeNull() + expect(safeHttpUrl(' javascript:alert(1)')).toBeNull() + }) + + it('FE-UTIL-SAFEURL-004: rejects the other schemes that are not a web page', () => { + expect(safeHttpUrl('data:text/html,')).toBeNull() + expect(safeHttpUrl('vbscript:msgbox(1)')).toBeNull() + expect(safeHttpUrl('file:///etc/passwd')).toBeNull() + }) + + it('FE-UTIL-SAFEURL-005: rejects a bare host, since window.open would treat it as a relative path', () => { + expect(safeHttpUrl('louvre.fr')).toBeNull() + expect(safeHttpUrl('//evil.example')).toBeNull() + }) + + it('FE-UTIL-SAFEURL-006: treats absent and empty as nothing to open', () => { + expect(safeHttpUrl(null)).toBeNull() + expect(safeHttpUrl(undefined)).toBeNull() + expect(safeHttpUrl('')).toBeNull() + }) +}) diff --git a/client/src/utils/safeUrl.ts b/client/src/utils/safeUrl.ts new file mode 100644 index 000000000..a89390fa1 --- /dev/null +++ b/client/src/utils/safeUrl.ts @@ -0,0 +1,18 @@ +import { placeWebsiteSchema } from '@trek/shared' + +/** + * Allow-list a URL before handing it to window.open. + * + * A place's website is a plain string in the older contracts, and it reaches + * window.open from the inspector, both sidebar context menus and the mobile + * place sheet. `window.open('javascript:…')` does not open a page — it evaluates + * the script in a fresh document that inherits this origin, so a co-traveller + * with place_edit could plant one and wait for someone to click Website. + * + * Server-side validation closes the write path. This closes the read path, which + * is what covers rows written before that validation existed and instances that + * have not updated yet — the same split as safeHexColor. + */ +export function safeHttpUrl(value: string | null | undefined): string | null { + return typeof value === 'string' && placeWebsiteSchema.safeParse(value).success ? value : null +} diff --git a/server/src/nest/places/places.controller.ts b/server/src/nest/places/places.controller.ts index 9d13543db..75f875fa2 100644 --- a/server/src/nest/places/places.controller.ts +++ b/server/src/nest/places/places.controller.ts @@ -20,7 +20,7 @@ import type { Response } from 'express'; import { isDemoWriteBlocked, DEMO_WRITE_ERROR } from '../common/demo-write'; import { RuntimeEnvService } from '../app-config/runtime-env.service'; import { memoryStorage } from 'multer'; -import { hexColorSchema } from '@trek/shared'; +import { hexColorSchema, placeImageUrlSchema, placeWebsiteSchema } from '@trek/shared'; import type { User } from '../../types'; import { PlacesService } from './places.service'; import { isUpdateConflict } from '../common/conflictResult'; @@ -63,6 +63,25 @@ function validateRouteColor(body: Record): void { } } +// Same reason as route_color: these two leave the database for a renderer that +// treats them as a URL — the thumbnail into marker HTML, the homepage into +// window.open — and the open record means the pipe never sees them. +function validateUrlFields(body: Record): void { + const image = body.image_url; + if (image !== undefined && image !== null) { + if (typeof image !== 'string' || !placeImageUrlSchema.safeParse(image).success) { + throw new HttpException({ error: 'image_url must be an uploaded path, a photo-proxy path, an inline image or an https URL' }, 400); + } + } + const website = body.website; + // '' is how the UI clears the field; treat it like an absent value. + if (website !== undefined && website !== null && website !== '') { + if (typeof website !== 'string' || !placeWebsiteSchema.safeParse(website).success) { + throw new HttpException({ error: 'website must be an http or https URL' }, 400); + } + } +} + function parseBool(v: unknown, defaultVal: boolean): boolean { return v === undefined || v === null ? defaultVal : String(v) === 'true'; } @@ -131,6 +150,7 @@ export class PlacesController { const trip = this.requireTrip(tripId, user); validateLengths(body); validateRouteColor(body); + validateUrlFields(body); this.requireEdit(trip, user); const place = this.places.create(tripId, body as never); this.places.broadcast(tripId, 'place:created', { place }, socketId); @@ -428,6 +448,7 @@ export class PlacesController { const trip = this.requireTrip(tripId, user); validateLengths(body); validateRouteColor(body); + validateUrlFields(body); this.requireEdit(trip, user); const result = this.places.update(tripId, id, body as never, ifMatch); if (!result) { diff --git a/server/src/nest/places/places.mcp.ts b/server/src/nest/places/places.mcp.ts index 39c88127f..ac95a64cc 100644 --- a/server/src/nest/places/places.mcp.ts +++ b/server/src/nest/places/places.mcp.ts @@ -5,6 +5,7 @@ import { demoDenied, ok, } from '@trek/nest-mcp'; import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; +import { placeWebsiteSchema } from '@trek/shared'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; import { AssignmentsService } from '../assignments/assignments.service'; @@ -63,7 +64,7 @@ export class PlacesMcp { google_ftid: z.string().optional().describe('Google Maps feature ID from search_place — enables direct Google Maps links'), osm_id: z.string().optional().describe('OpenStreetMap ID from search_place (e.g. "way:12345") — enables opening hours if no Google ID'), notes: z.string().max(2000).optional(), - website: z.string().max(500).optional(), + website: placeWebsiteSchema.optional(), phone: z.string().max(50).optional(), price: z.number().nonnegative().optional().describe('Cost of this place/activity (e.g. ticket price, entry fee)'), currency: z.string().length(3).optional().describe('ISO 4217 currency code (e.g. "EUR", "USD")'), @@ -103,7 +104,7 @@ export class PlacesMcp { google_ftid: z.string().optional().describe('Google Maps feature ID from search_place — enables direct Google Maps links'), osm_id: z.string().optional().describe('OpenStreetMap ID from search_place (e.g. "way:12345")'), place_notes: z.string().max(2000).optional().describe('Notes for the place'), - website: z.string().max(500).optional(), + website: placeWebsiteSchema.optional(), phone: z.string().max(50).optional(), assignment_notes: z.string().max(500).optional().describe('Notes for this day assignment'), price: z.number().nonnegative().optional().describe('Cost of this place/activity (e.g. ticket price, entry fee)'), @@ -158,7 +159,7 @@ export class PlacesMcp { end_time: z.string().max(50).optional().describe('End time (e.g. "11:00")'), duration_minutes: z.number().int().positive().optional(), notes: z.string().max(2000).optional(), - website: z.string().max(500).optional(), + website: placeWebsiteSchema.optional(), phone: z.string().max(50).optional(), transport_mode: z.enum(['walking', 'driving', 'cycling', 'transit', 'flight']).optional(), osm_id: z.string().optional().describe('OpenStreetMap ID (e.g. "way:12345")'), @@ -366,7 +367,7 @@ export class PlacesMcp { end_time: z.string().max(50).optional().describe('End time (e.g. "11:00")'), duration_minutes: z.number().int().positive().optional(), notes: z.string().max(2000).optional(), - website: z.string().max(500).optional(), + website: placeWebsiteSchema.optional(), phone: z.string().max(50).optional(), description: z.string().max(2000).optional(), }, diff --git a/server/src/nest/places/places.rpc.ts b/server/src/nest/places/places.rpc.ts index 3f61da812..91f20afa6 100644 --- a/server/src/nest/places/places.rpc.ts +++ b/server/src/nest/places/places.rpc.ts @@ -1,4 +1,4 @@ -import { placeCreateRequestSchema, placeUpdateRequestSchema } from '@trek/shared'; +import { placeCreateRequestSchema, placeImageUrlSchema, placeUpdateRequestSchema, placeWebsiteSchema } from '@trek/shared'; import { PluginController, PluginMethod } from '../plugins/host/rpc-kit/decorators'; import { PluginGuards } from '../plugins/host/plugin-guards.service'; import { BadParams, ForbiddenResource } from '../plugins/host/rpc-errors'; @@ -18,6 +18,23 @@ const PLACE_EDIT_ACTION = 'place_edit'; */ const PLACE_STR_LIMITS: Record = { name: 200, description: 2000, address: 500, notes: 2000 }; +/** + * The two URL fields the REST controller pins in validateUrlFields. Same reason + * they are checked there rather than in the contract: the write schemas are open + * records, so a plugin could otherwise store a thumbnail or a homepage the web + * app refuses — and both end up somewhere that treats them as a URL. + */ +function capUrls(input: Record): void { + const image = input.image_url; + if (image !== undefined && image !== null && !placeImageUrlSchema.safeParse(image).success) { + throw new BadParams('invalid place: image_url must be an uploaded path, a photo-proxy path, an inline image or an https URL'); + } + const website = input.website; + if (website !== undefined && website !== null && website !== '' && !placeWebsiteSchema.safeParse(website).success) { + throw new BadParams('invalid place: website must be an http or https URL'); + } +} + /** * The place surface a plugin may reach (#plugins). * @@ -41,6 +58,7 @@ export class PlacesRpc { const parsed = placeCreateRequestSchema.safeParse(params.input); if (!parsed.success) throw new BadParams(`invalid place: ${schemaMessage(parsed.error)}`); this.guards.capStrings(parsed.data as Record, PLACE_STR_LIMITS); + capUrls(parsed.data as Record); this.guards.requireTripEdit(tripId, actor, PLACE_EDIT_ACTION); const place = this.places.create(String(tripId), parsed.data as unknown as PlaceCreateInput); this.realtime.broadcast(tripId, 'place:created', { place }); @@ -56,6 +74,7 @@ export class PlacesRpc { const parsed = placeUpdateRequestSchema.safeParse(params.input); if (!parsed.success) throw new BadParams(`invalid place: ${schemaMessage(parsed.error)}`); this.guards.capStrings(parsed.data as Record, PLACE_STR_LIMITS); + capUrls(parsed.data as Record); this.guards.requireTripEdit(tripId, actor, PLACE_EDIT_ACTION); const place = this.places.update(String(tripId), String(placeId), parsed.data as PlaceUpdateInput); if (place === null) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); diff --git a/server/tests/unit/nest/places.controller.test.ts b/server/tests/unit/nest/places.controller.test.ts index 12f4af731..71cfcae9b 100644 --- a/server/tests/unit/nest/places.controller.test.ts +++ b/server/tests/unit/nest/places.controller.test.ts @@ -301,6 +301,59 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou }); }); + // image_url and website leave the database for something that treats them as a + // URL — the thumbnail into hand-built marker HTML, the homepage into + // window.open. The write body is an open record, so the Zod pipe never sees + // either one; they are checked here for the same reason route_color is. + describe('image_url and website', () => { + const imageErr = { status: 400, body: { error: 'image_url must be an uploaded path, a photo-proxy path, an inline image or an https URL' } }; + const siteErr = { status: 400, body: { error: 'website must be an http or https URL' } }; + const ctl = (over: Partial = {}) => new PlacesController(svc(over), new RuntimeEnvService()); + + it('400s an image_url the marker builders were never meant to receive, before the permission check', () => { + const canEdit = vi.fn().mockReturnValue(false); // would 403 if it got that far + for (const image_url of [ + 'javascript:alert(1)', + 'data:text/html,', + 'http://insecure.example/p.png', + '//evil.example/p.png', + 123, + ]) { + expect(thrown(() => ctl({ canEdit }).update(user, '5', '9', { image_url }))).toEqual(imageErr); + } + expect(thrown(() => ctl({ canEdit }).create(user, '5', { name: 'T', image_url: 'javascript:alert(1)' }))).toEqual(imageErr); + expect(canEdit).not.toHaveBeenCalled(); + }); + + it('keeps accepting every shape the app actually stores', () => { + const update = vi.fn().mockReturnValue({ id: 9 }); + for (const image_url of [ + '/uploads/places/eiffel.jpg', + '/api/maps/place-photo/abc123', + 'data:image/jpeg;base64,/9j/4AAQSkZJRg==', + 'https://images.example/photo.jpg', + null, + ]) { + expect(ctl({ update } as Partial).update(user, '5', '9', { image_url })).toEqual({ place: { id: 9 } }); + } + }); + + it('400s a website that window.open would not treat as a page', () => { + const canEdit = vi.fn().mockReturnValue(false); + for (const website of ['javascript:fetch("/api/trips")', 'data:text/html,x', 'louvre.fr', 42]) { + expect(thrown(() => ctl({ canEdit }).update(user, '5', '9', { website }))).toEqual(siteErr); + } + expect(canEdit).not.toHaveBeenCalled(); + }); + + it('accepts http and https, and treats the empty string as clearing the field', () => { + const update = vi.fn().mockReturnValue({ id: 9 }); + for (const website of ['https://louvre.fr', 'http://pension.at', '', null]) { + expect(ctl({ update } as Partial).update(user, '5', '9', { website })).toEqual({ place: { id: 9 } }); + } + }); + }); + it('PUT /:id forwards the base-version token and 409s on a conflict (#1135)', () => { const update = vi.fn().mockReturnValue({ conflict: true, server: { id: 9, name: 'Theirs' } }); const onUpdated = vi.fn(); const broadcast = vi.fn(); diff --git a/shared/src/collection/collection.schema.ts b/shared/src/collection/collection.schema.ts index 894fef266..d5379d5fe 100644 --- a/shared/src/collection/collection.schema.ts +++ b/shared/src/collection/collection.schema.ts @@ -1,4 +1,4 @@ -import { placeCategorySchema, placeRatingVoteSchema } from '../place/place.schema'; +import { placeCategorySchema, placeImageUrlSchema, placeRatingVoteSchema, placeWebsiteSchema } from '../place/place.schema'; import { tagSchema } from '../tag/tag.schema'; import { z } from 'zod'; @@ -53,6 +53,9 @@ export const collectionPlaceSchema = z.object({ price: z.number().nullable().optional(), currency: z.string().nullable().optional(), notes: z.string().nullable().optional(), + // Deliberately open on the way out, pinned on the way in (see the save/update + // request schemas below): rows written before the request-side check existed + // still have to round-trip. image_url: z.string().nullable().optional(), google_place_id: z.string().nullable().optional(), google_ftid: z.string().nullable().optional(), @@ -150,11 +153,11 @@ export const collectionSavePlaceRequestSchema = z.object({ price: z.number().nullable().optional(), currency: z.string().nullable().optional(), notes: z.string().nullable().optional(), - image_url: z.string().nullable().optional(), + image_url: placeImageUrlSchema.nullable().optional(), google_place_id: z.string().nullable().optional(), google_ftid: z.string().nullable().optional(), osm_id: z.string().nullable().optional(), - website: z.string().nullable().optional(), + website: placeWebsiteSchema.nullable().optional(), phone: z.string().nullable().optional(), status: collectionStatusSchema.optional(), links: collectionLinksSchema.optional(), @@ -203,7 +206,7 @@ export const collectionPlaceUpdateRequestSchema = z.object({ // Replace the place's per-collection label assignments (omit to leave unchanged). label_ids: z.array(z.number()).optional(), // Custom thumbnail (#1136): null clears it (falls back to the auto-fetched photo). - image_url: z.string().nullable().optional(), + image_url: placeImageUrlSchema.nullable().optional(), }); export type CollectionPlaceUpdateRequest = z.infer; diff --git a/shared/src/place/place.schema.ts b/shared/src/place/place.schema.ts index 9c3fa5379..d0d63da28 100644 --- a/shared/src/place/place.schema.ts +++ b/shared/src/place/place.schema.ts @@ -19,6 +19,34 @@ const open = z.record(z.string(), z.unknown()); /** `#rgb` / `#rrggbb`, the form both map renderers and CSS accept. */ export const hexColorSchema = z.string().regex(/^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/); +/** + * The four shapes a place thumbnail is ever allowed to take: a file we stored, a + * photo-proxy path, an inline thumbnail, or a remote https image. + * + * The map markers build their HTML as a string, so what lands here is one input + * away from a renderer that forgets to escape. Pinning the scheme on the way in + * means a stored value cannot carry anything a future consumer has to defend + * against — the escaping in the marker builders stops being the only thing + * standing between the database and the DOM. + */ +export const placeImageUrlSchema = z.string().max(2048).refine( + v => + v.startsWith('/uploads/') + || v.startsWith('/api/maps/place-photo/') + || /^data:image\/(png|jpe?g|webp|gif|avif);base64,/i.test(v) + || /^https:\/\//i.test(v), + { message: 'must be an uploaded path, a photo-proxy path, an inline image or an https URL' }, +); + +/** + * A place's homepage. It reaches window.open() on the client, where a + * javascript: value would run in this origin rather than opening a page. + */ +export const placeWebsiteSchema = z.string().max(500).refine( + v => /^https?:\/\//i.test(v), + { message: 'must be an http or https URL' }, +); + /** * Embedded category as returned on a place — a trimmed projection of the * categories row (id/name/color/icon), built inline by placeService and