From 3e0bfbbb2949085484072190fdf98011156deeb0 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 23 Aug 2026 10:38:23 +0200 Subject: [PATCH] fix(client): give the desktop halves the same fixes their mobile twins got Several of the fixes in this branch landed on a mobile screen while the desktop component with the identical bug sat in another package and was left alone, which leaves the two disagreeing about the same handler. So: the appearance tab flushes its debounced write on unmount, the journey share copy guards navigator.clipboard (undefined on a plain-HTTP self-hosted install), the admin collab toggle rolls back from a scoped snapshot instead of a stale one, the reservation modal parses metadata through the guarded helper rather than a bare JSON.parse that takes the route down, and the notification toggle surfaces its failures and stops racing concurrent toggles. The Journey components reached the memories provider-status endpoint with a raw fetch(), past every apiClient interceptor; they go through a small memoriesApi now. And the server half of the OSM menu_url finding: the client renders it through safeHttpUrl, so place-enrichment should not be handing a javascript: value out in the first place. The share section hides itself when the server says the link is not this user's to manage, rather than offering a create button that 403s. --- client/src/api/client.surface.test.ts | 18 ++++++- client/src/api/client.ts | 30 +++++++++--- .../JourneyDetailPageEntryEditor.test.tsx | 33 +++++++++++-- .../Journey/JourneyDetailPageEntryEditor.tsx | 37 +++++++++----- .../Journey/JourneyDetailPageGalleryView.tsx | 9 ++-- .../JourneyDetailPageProviderPicker.tsx | 33 +++++-------- .../Journey/JourneyShareSection.test.tsx | 34 ++++++++++++- .../Journey/JourneyShareSection.tsx | 37 +++++++++++--- .../Planner/ReservationModal.test.tsx | 23 +++++++++ .../components/Planner/ReservationModal.tsx | 3 +- .../Settings/AppearanceSettingsTab.test.tsx | 31 ++++++++++++ .../Settings/AppearanceSettingsTab.tsx | 12 ++++- .../Settings/NotificationsTab.test.tsx | 48 +++++++++++++++++++ .../components/Settings/NotificationsTab.tsx | 8 +++- client/src/pages/AdminPage.test.tsx | 41 ++++++++++++++-- client/src/pages/AdminPage.tsx | 12 +++-- client/src/pages/JourneyDetailPage.test.tsx | 4 +- .../pages/JourneyDetailPage.wiring.test.tsx | 2 +- client/src/pages/admin/useAdmin.test.tsx | 41 ++++++++++++++-- client/src/pages/admin/useAdmin.ts | 34 ++++++++++++- .../place-enrichment.service.ts | 8 +++- .../nest/place-enrichment.service.test.ts | 13 +++++ 22 files changed, 435 insertions(+), 76 deletions(-) diff --git a/client/src/api/client.surface.test.ts b/client/src/api/client.surface.test.ts index e3d76cb2b..bca591154 100644 --- a/client/src/api/client.surface.test.ts +++ b/client/src/api/client.surface.test.ts @@ -1,4 +1,4 @@ -// FE-APISURF-001 to FE-APISURF-052 +// FE-APISURF-001 to FE-APISURF-054 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' import type { AxiosResponse } from 'axios' import { http, HttpResponse } from 'msw' @@ -9,7 +9,7 @@ import { tagsApi, categoriesApi, adminApi, addonsApi, pluginsApi, airtrailApi, journeyApi, mapsApi, airportsApi, budgetApi, filesApi, reservationsApi, healthApi, weatherApi, configApi, helpApi, settingsApi, accommodationsApi, dayNotesApi, collabApi, backupApi, - shareApi, transitApi, tripInviteApi, notificationsApi, inAppNotificationsApi, + shareApi, transitApi, tripInviteApi, notificationsApi, inAppNotificationsApi, memoriesApi, } from './client' interface Recorded { method: string; url: string; body: unknown } @@ -396,6 +396,20 @@ describe('client > endpoint wiring', () => { ]) }) + it('FE-APISURF-053: memoriesApi maps the photo-provider endpoints', async () => { + await assertCalls([ + { n: 'status', r: () => memoriesApi.status('immich'), e: 'GET /api/integrations/memories/immich/status' }, + { n: 'search', r: () => memoriesApi.search('immich', { from: '2026-01-01', to: '2026-01-02', page: 1, size: 50 }), e: 'POST /api/integrations/memories/immich/search' }, + { n: 'albums', r: () => memoriesApi.albums('immich'), e: 'GET /api/integrations/memories/immich/albums' }, + { n: 'albumPhotos', r: () => memoriesApi.albumPhotos('immich', 'alb-1'), e: 'GET /api/integrations/memories/immich/albums/alb-1/photos' }, + ]) + }) + + it('FE-APISURF-054: memoriesApi passes the album passphrase as a query parameter', async () => { + const rec = await traceOne(() => memoriesApi.albumPhotos('synologyphotos', 'alb-2', 'p/w?')) + expect(rec.url).toBe('/api/integrations/memories/synologyphotos/albums/alb-2/photos?passphrase=p%2Fw%3F') + }) + it('FE-APISURF-016: mapsApi and airportsApi map the geo endpoints', async () => { await assertCalls([ { n: 'maps.search', r: () => mapsApi.search('Rome'), e: 'POST /api/maps/search' }, diff --git a/client/src/api/client.ts b/client/src/api/client.ts index 79aa8de2f..c6bc1d065 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -55,6 +55,7 @@ import { } from '@trek/shared' import { getSocketId } from './websocket' import { probeNow } from '../sync/connectivity' +import { downloadBlob } from '../utils/fileDownload' /** * Validate a response payload against its @trek/shared Zod schema — but only in @@ -968,6 +969,27 @@ export const journeyApi = { getPublicJourney: (token: string) => apiClient.get(`/public/journey/${token}`).then(r => r.data), } +// Photo providers (Immich, Synology Photos, …) behind /api/integrations/memories. +// The provider sits on the user's own hardware and the server proxies through to +// it, so these get a longer leash than the 8s default — a cold Immich answering a +// date-range search regularly needs more. +const MEMORIES_TIMEOUT = 20000 + +export const memoriesApi = { + status: (provider: string): Promise<{ connected: boolean }> => + apiClient.get(`/integrations/memories/${provider}/status`, { timeout: MEMORIES_TIMEOUT }).then(r => r.data), + search: (provider: string, body: { from: string; to: string; page: number; size: number }, signal?: AbortSignal) => + apiClient.post(`/integrations/memories/${provider}/search`, body, { timeout: MEMORIES_TIMEOUT, signal }).then(r => r.data), + albums: (provider: string) => + apiClient.get(`/integrations/memories/${provider}/albums`, { timeout: MEMORIES_TIMEOUT }).then(r => r.data), + albumPhotos: (provider: string, albumId: string, passphrase?: string, signal?: AbortSignal) => + apiClient.get(`/integrations/memories/${provider}/albums/${albumId}/photos`, { + timeout: MEMORIES_TIMEOUT, + params: passphrase ? { passphrase } : undefined, + signal, + }).then(r => r.data), +} + export const mapsApi = { search: (query: string, lang?: string) => apiClient.post(`/maps/search?lang=${lang || 'en'}`, { query }).then(r => checkInDev(mapsSearchResultSchema, r.data, 'maps.search')), autocomplete: (input: string, lang?: string, locationBias?: { low: { lat: number; lng: number }; high: { lat: number; lng: number } }, signal?: AbortSignal, sessionToken?: string) => @@ -1144,13 +1166,7 @@ export const backupApi = { credentials: 'include', }) if (!res.ok) throw new Error('Download failed') - const blob = await res.blob() - const url = URL.createObjectURL(blob) - const a = document.createElement('a') - a.href = url - a.download = filename - a.click() - URL.revokeObjectURL(url) + downloadBlob(await res.blob(), filename) }, delete: (filename: string) => apiClient.delete(`/backup/${filename}`).then(r => r.data), restore: (filename: string) => apiClient.post(`/backup/restore/${filename}`).then(r => r.data), diff --git a/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx b/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx index 9a5f0e37e..725483433 100644 --- a/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx +++ b/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx @@ -586,11 +586,11 @@ describe('EntryEditor', () => { expect(stripImg.getAttribute('src')).toBe('/api/photos/100/original') }) - it('FE-JRN-EDITOR-029: reorders photos locally even when persisting the order fails', async () => { + it('FE-JRN-EDITOR-029: rolls the order back when persisting it fails', async () => { let attempts = 0 server.use(http.patch('/api/journeys/photos/:id', () => { attempts += 1 - return new HttpResponse(null, { status: 500 }) + return HttpResponse.json({ error: 'sort rejected' }, { status: 500 }) })) const user = userEvent.setup() const { container } = mountEditor(buildEntry({ id: 10, photos: [buildPhoto(100), buildPhoto(101)] })) @@ -598,8 +598,9 @@ describe('EntryEditor', () => { await user.click(screen.getByRole('button', { name: 'Make 1st' })) await waitFor(() => expect(attempts).toBe(2)) + await waitFor(() => expect(toastSpy).toHaveBeenCalledWith('sort rejected', 'error', undefined)) const order = Array.from(container.querySelectorAll('.w-20.h-20 img')).map(i => i.getAttribute('src')) - expect(order).toEqual(['/api/photos/101/thumbnail', '/api/photos/100/thumbnail']) + expect(order).toEqual(['/api/photos/100/thumbnail', '/api/photos/101/thumbnail']) }) it('FE-JRN-EDITOR-030: dropping an unsaved gallery pick cancels its link', async () => { @@ -865,6 +866,32 @@ describe('EntryEditor', () => { expect(onSave.mock.calls[0][0]).toEqual(expect.objectContaining({ entry_time: null })) }) + it('FE-JRN-EDITOR-044: reports a rejected save and keeps the editor open', async () => { + const user = userEvent.setup() + const { onSave, onDone } = mountEditor(buildEntry({ id: 10, title: 'Old' })) + onSave.mockRejectedValueOnce(new Error('server said no')) + + await user.click(screen.getByRole('button', { name: 'Save' })) + + await waitFor(() => expect(toastSpy).toHaveBeenCalledWith('server said no', 'error', undefined)) + expect(onDone).not.toHaveBeenCalled() + expect(screen.getByRole('heading', { name: 'Edit Entry' })).toBeInTheDocument() + }) + + it('FE-JRN-EDITOR-046: gives the preview blob URLs back when the editor closes', async () => { + const revoke = vi.spyOn(URL, 'revokeObjectURL') + const { container, unmount } = mountEditor(buildEntry()) + + fireEvent.change(container.querySelector('input[type="file"]') as HTMLInputElement, { + target: { files: [new File(['a'], 'a.jpg', { type: 'image/jpeg' })] }, + }) + await waitFor(() => expect(container.querySelector('img[src="blob:preview"]')).toBeInTheDocument()) + + unmount() + expect(revoke).toHaveBeenCalledWith('blob:preview') + revoke.mockRestore() + }) + it('FE-JRN-EDITOR-043: the camera route is its own input, so the library picker survives', () => { const { container } = mountEditor(buildEntry({ id: 10 })) diff --git a/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx b/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx index 19f57f0ab..8c6e15bdb 100644 --- a/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx +++ b/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx @@ -1,9 +1,9 @@ -import { useEffect, useState, useRef } from 'react' +import { useEffect, useMemo, useState, useRef } from 'react' import { X, Plus, Image, Minus, Check, MapPin, Locate, Camera } from 'lucide-react' import { normalizeImageFiles } from '../../utils/convertHeic' import { type ResilientResult, type UploadProgress } from '../../utils/uploadQueue' import { useTranslation } from '../../i18n' -import { journeyApi, mapsApi, addonsApi } from '../../api/client' +import { journeyApi, mapsApi, addonsApi, memoriesApi } from '../../api/client' import { useToast } from '../shared/Toast' import { getCurrentPositionOnce } from '../../hooks/useGeolocation' import { getApiErrorMessage } from '../../types' @@ -52,6 +52,10 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips, const [uploadProgress, setUploadProgress] = useState<{ done: number; total: number } | null>(null) const [photos, setPhotos] = useState<(JourneyPhoto | GalleryPhoto)[]>(entry.photos || []) const [pendingFiles, setPendingFiles] = useState([]) + // Minting the preview URL inline in the JSX would hand out a fresh blob on + // every keystroke in the story field and never give one back. + const pendingUrls = useMemo(() => pendingFiles.map(f => URL.createObjectURL(f)), [pendingFiles]) + useEffect(() => () => { pendingUrls.forEach(u => URL.revokeObjectURL(u)) }, [pendingUrls]) const [pendingLinkIds, setPendingLinkIds] = useState([]) const [showGalleryPick, setShowGalleryPick] = useState(false) const [photoTab, setPhotoTab] = useState<'upload' | 'gallery' | 'external'>('upload') @@ -103,8 +107,7 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips, const connected: { id: string; name: string }[] = [] for (const provider of enabled) { try { - const response = await fetch(`/api/integrations/memories/${provider.id}/status`, { credentials: 'include' }) - if (response.ok && (await response.json()).connected) connected.push({ id: provider.id, name: provider.name }) + if ((await memoriesApi.status(provider.id)).connected) connected.push({ id: provider.id, name: provider.name }) } catch {} } setAvailableProviders(connected) @@ -186,6 +189,11 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips, setPendingProviderGroups([]) } onDone() + } catch (err) { + // Neither the page callback nor journeyStore toasts, so without this the + // whole entry just fails to save with no sign of it. + toast.error(getApiErrorMessage(err, t('journey.settings.saveFailed'))) + return } finally { setSaving(false) } @@ -436,13 +444,18 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips, + ), })); @@ -1569,8 +1574,38 @@ describe('AdminPage', () => { fireEvent.click(screen.getByRole('button', { name: /toggle chat/i })); - // The rollback re-renders the stub; the addon panel stays mounted either way - await waitFor(() => expect(screen.getByTestId('addon-manager')).toBeInTheDocument()); + await waitFor(() => expect(screen.getByTestId('collab-chat-state')).toHaveTextContent('true')); + }); + + // A snapshot-wide rollback would undo the second toggle as well, leaving the + // panel disagreeing with the server until a reload. + it('a failing collab toggle keeps a second toggle made while it was in flight', async () => { + server.use( + http.get('/api/admin/collab-features', () => + HttpResponse.json({ chat: true, notes: true, polls: true, whatsnext: true }) + ), + http.put('/api/admin/collab-features', async ({ request }) => { + const body = (await request.json()) as Record; + if ('chat' in body) { + await delay(30); + return HttpResponse.json({}, { status: 500 }); + } + return HttpResponse.json({ success: true }); + }) + ); + + seedStore(useAuthStore, { isAuthenticated: true, user: buildAdmin() }); + render(); + + await waitFor(() => expect(screen.getByRole('button', { name: /^users$/i })).toBeInTheDocument()); + fireEvent.click(screen.getByRole('button', { name: /^addons$/i })); + await waitFor(() => expect(screen.getByTestId('collab-chat-state')).toHaveTextContent('true')); + + fireEvent.click(screen.getByRole('button', { name: /toggle chat/i })); + fireEvent.click(screen.getByRole('button', { name: /toggle notes/i })); + + await waitFor(() => expect(screen.getByTestId('collab-chat-state')).toHaveTextContent('true')); + expect(screen.getByTestId('collab-notes-state')).toHaveTextContent('false'); }); }); diff --git a/client/src/pages/AdminPage.tsx b/client/src/pages/AdminPage.tsx index 16e31f8a6..60f511300 100644 --- a/client/src/pages/AdminPage.tsx +++ b/client/src/pages/AdminPage.tsx @@ -161,9 +161,15 @@ function AdminPageDesktop(): React.ReactElement { setBagTrackingEnabled(next) try { await adminApi.updateBagTracking(next) } catch { setBagTrackingEnabled(!next) } }} collabFeatures={collabFeatures} onToggleCollabFeature={async (key: string) => { - const next = { ...collabFeatures, [key]: !collabFeatures[key] } - setCollabFeatures(next) - try { await adminApi.updateCollabFeatures({ [key]: next[key] }) } catch { setCollabFeatures(collabFeatures) } + const previous = collabFeatures[key] + setCollabFeatures({ ...collabFeatures, [key]: !previous }) + try { + await adminApi.updateCollabFeatures({ [key]: !previous }) + } catch { + // Only this key rolls back — a slower request must not undo a toggle + // the admin made in the meantime. + setCollabFeatures(prev => ({ ...prev, [key]: previous })) + } }} /> )} diff --git a/client/src/pages/JourneyDetailPage.test.tsx b/client/src/pages/JourneyDetailPage.test.tsx index 4e54ca9b5..3c116a745 100644 --- a/client/src/pages/JourneyDetailPage.test.tsx +++ b/client/src/pages/JourneyDetailPage.test.tsx @@ -3720,13 +3720,15 @@ describe('JourneyDetailPage', () => { it('clicking Copy on share link copies to clipboard', async () => { const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); - // Mock clipboard + // Mock clipboard. jsdom leaves isSecureContext undefined, which the copy + // handler reads as plain HTTP and answers with its execCommand fallback. const mockWriteText = vi.fn().mockResolvedValue(undefined); Object.defineProperty(navigator, 'clipboard', { value: { writeText: mockWriteText }, writable: true, configurable: true, }); + Object.defineProperty(window, 'isSecureContext', { value: true, writable: true, configurable: true }); server.use( http.get('/api/journeys/1/share-link', () => { diff --git a/client/src/pages/JourneyDetailPage.wiring.test.tsx b/client/src/pages/JourneyDetailPage.wiring.test.tsx index 6d867427e..8947886df 100644 --- a/client/src/pages/JourneyDetailPage.wiring.test.tsx +++ b/client/src/pages/JourneyDetailPage.wiring.test.tsx @@ -98,7 +98,7 @@ function buildHook(over: Record = {}): Record unlinkTrip: null, setUnlinkTrip: vi.fn(), showSettings: false, setShowSettings: vi.fn(), hideSkeletons: false, setHideSkeletons: vi.fn(), - mapRef: { current: null }, fullMapRef: { current: null }, + mapRef: { current: null }, fullMapRef: { current: null }, galleryUploadRef: { current: null }, activeLocationId: null, handleMarkerClick: vi.fn(), handleLocationClick: vi.fn(), mapEntries: [], sidebarMapItems: [], tripDates: new Set(), isMobile: false, feedEdge: { atTop: true, atBottom: true }, scrollFeedTo: vi.fn(), diff --git a/client/src/pages/admin/useAdmin.test.tsx b/client/src/pages/admin/useAdmin.test.tsx index dc0f31c4b..00fec9a89 100644 --- a/client/src/pages/admin/useAdmin.test.tsx +++ b/client/src/pages/admin/useAdmin.test.tsx @@ -1,4 +1,4 @@ -// FE-ADMHOOK-001 to FE-ADMHOOK-045 +// FE-ADMHOOK-001 to FE-ADMHOOK-049 import { http, HttpResponse } from 'msw'; import React from 'react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -32,8 +32,13 @@ function wrapper({ children }: { children: React.ReactNode }) { } /** Mounts the hook and waits until the initial parallel loads have settled. */ -async function mountAdmin() { - const view = renderHook(() => useAdmin(), { wrapper }); +async function mountAdmin(url = '/admin') { + const at = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + const view = renderHook(() => useAdmin(), { wrapper: url === '/admin' ? wrapper : at }); await waitFor(() => expect(view.result.current.isLoading).toBe(false)); return view; } @@ -732,6 +737,36 @@ describe('useAdmin', () => { expect(result.current.users).toHaveLength(2); }); + it('FE-ADMHOOK-046: opens on the tab the URL asks for', async () => { + const { result } = await mountAdmin('/admin?tab=audit'); + expect(result.current.activeTab).toBe('audit'); + }); + + /* + * A tab id with no panel behind it — a typo, or a link from an install that + * has tabs this one does not — used to open the page on an empty content + * area with nothing selected in the sidebar. + */ + it('FE-ADMHOOK-047: falls back to users when the URL names a tab that does not exist', async () => { + const { result } = await mountAdmin('/admin?tab=nonsense'); + expect(result.current.activeTab).toBe('users'); + }); + + /* + * The sidebar entries are built conditionally but the panels are not, so a + * hosted install had its hidden panels one URL away. + */ + it('FE-ADMHOOK-048: refuses a tab a managed install hides', async () => { + seedStore(useAuthStore, { isAuthenticated: true, user: me, managed: true }); + const { result } = await mountAdmin('/admin?tab=backup'); + await waitFor(() => expect(result.current.activeTab).toBe('users')); + }); + + it('FE-ADMHOOK-049: keeps that same tab on an install that runs itself', async () => { + const { result } = await mountAdmin('/admin?tab=backup'); + expect(result.current.activeTab).toBe('backup'); + }); + it('FE-ADMHOOK-044: mcpEnabled follows the addon store', async () => { const { result } = await mountAdmin(); diff --git a/client/src/pages/admin/useAdmin.ts b/client/src/pages/admin/useAdmin.ts index 4b2d6d8dc..bc03d9ee4 100644 --- a/client/src/pages/admin/useAdmin.ts +++ b/client/src/pages/admin/useAdmin.ts @@ -7,8 +7,32 @@ import { useAddonStore } from '../../store/addonStore' import { useTranslation } from '../../i18n' import { getApiErrorMessage } from '../../types' import { useToast } from '../../components/shared/Toast' +import { managedAdminTabs } from '../../managed' import type { AdminUser, AdminStats, OidcConfig, UpdateInfo } from './adminModel' +/** + * Every tab id AdminPage can render a panel for, whatever this install offers. + * + * ?tab= is user input and was taken as read: an id with no panel behind it — + * a typo, or a link from an install that has tabs this one does not — opened + * the admin page on an empty content area with nothing selected in the + * sidebar. Labels and icons stay in AdminPage; this is only the vocabulary. + */ +const ADMIN_TAB_IDS = [ + 'users', 'defaults', 'config', 'settings', 'addons', 'plugins', 'storage', + 'notifications', 'mcp-tokens', 'github', 'backup', 'audit', 'dev-notifications', +] + +/** + * The ones a managed install deliberately does not offer. + * + * The sidebar entries are built conditionally, but the panels are not, so + * ?tab=backup opened a backup schedule that competes with the real one on a + * hosted instance. Checked in an effect rather than in the initial state + * because `managed` arrives with /app-config, after the first render. + */ +const MANAGED_HIDDEN = ['storage', 'github', 'backup'] + /** * Admin page logic — owns every admin data slice (users, stats, invites, auth * toggles, OIDC, feature flags, API keys, SMTP, version/update) plus the CRUD @@ -29,7 +53,11 @@ export function useAdmin() { // page with eleven of them. Read once for the initial value and written back // on every change, so the address bar keeps saying where the reader is. const [searchParams, setSearchParams] = useSearchParams() - const [activeTab, setActiveTabState] = useState(() => searchParams.get('tab') || 'users') + const [activeTab, setActiveTabState] = useState(() => { + const asked = searchParams.get('tab') + const known = [...ADMIN_TAB_IDS, ...managedAdminTabs.map(tab => tab.id)] + return asked && known.includes(asked) ? asked : 'users' + }) const setActiveTab = useCallback((tab: string) => { setActiveTabState(tab) // replace, not push: eleven tabs would otherwise fill the back button with @@ -41,6 +69,10 @@ export function useAdmin() { return next }, { replace: true }) }, [setSearchParams]) + useEffect(() => { + if (managed && MANAGED_HIDDEN.includes(activeTab)) setActiveTab('users') + }, [managed, activeTab, setActiveTab]) + const [users, setUsers] = useState([]) const [stats, setStats] = useState(null) const [isLoading, setIsLoading] = useState(true) diff --git a/server/src/nest/place-enrichment/place-enrichment.service.ts b/server/src/nest/place-enrichment/place-enrichment.service.ts index 7633064f2..4aa5a2acb 100644 --- a/server/src/nest/place-enrichment/place-enrichment.service.ts +++ b/server/src/nest/place-enrichment/place-enrichment.service.ts @@ -1,5 +1,6 @@ import { createHash } from 'node:crypto'; import { Injectable } from '@nestjs/common'; +import { placeWebsiteSchema } from '@trek/shared'; import type { MapsPlaceEnrichmentRequest, MapsPlaceEnrichmentResult, @@ -173,8 +174,11 @@ export function collectFacts(details: Record | null): PlaceFact // OSM writes several cuisines semicolon-separated and underscored. if (cuisine) push('cuisine', cuisine.split(';').map((c) => c.replace(/_/g, ' ').trim()).filter(Boolean).join(', ')); - const menu = typeof details.menu_url === 'string' ? details.menu_url : null; - if (menu) push('menu', null, menu); + // `menu_url` is a community-editable OSM tag that becomes an href on the + // client, so it goes through the same allow-list as a place's website — + // anything but http(s) is dropped rather than rendered as a link. + const menu = typeof details.menu_url === 'string' ? details.menu_url.trim() : ''; + if (placeWebsiteSchema.safeParse(menu).success) push('menu', null, menu); if (yesNo(details.outdoor_seating) === 'yes') push('outdoorSeating', null); if (yesNo(details.takeaway) === 'yes') push('takeaway', null); diff --git a/server/tests/unit/nest/place-enrichment.service.test.ts b/server/tests/unit/nest/place-enrichment.service.test.ts index ba1e62555..dca411403 100644 --- a/server/tests/unit/nest/place-enrichment.service.test.ts +++ b/server/tests/unit/nest/place-enrichment.service.test.ts @@ -729,6 +729,19 @@ describe('collectFacts', () => { expect(facts.find((f) => f.kind === 'internetAccess')).toEqual({ kind: 'internetAccess', value: null, url: null }); }); + it('ENRICH-052c: drops a menu_url that is not http(s)', () => { + // Anyone can edit the OSM tag and the client turns it into an href. + for (const menu_url of ['javascript:alert(1)', 'data:text/html,', 'ftp://host/menu', '']) { + expect(collectFacts(osm({ menu_url })).map((f) => f.kind)).toEqual([]); + } + }); + + it('ENRICH-052d: keeps an http menu_url and trims the surrounding whitespace', () => { + expect(collectFacts(osm({ menu_url: ' http://example.org/menu ' }))).toEqual([ + { kind: 'menu', value: null, url: 'http://example.org/menu' }, + ]); + }); + it('ENRICH-053: takes no chips at all from a Google place', () => { expect(collectFacts(null)).toEqual([]); // cuisine/outdoor_seating are OSM tagging and never appear on a Google blob;