From f24d44b4a350548a75e3397e50d1c13e6e591315 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 28 Jun 2026 20:06:34 +0200 Subject: [PATCH] feat(trips): download chosen Unsplash covers into uploads (#1277) Previously a selected Unsplash photo was stored as a remote images.unsplash.com hot-link, so covers broke offline and on link rot. The trip PUT handler now fetches the picked image through the SSRF guard and saves it under uploads/covers, rewriting cover_image to the local path (502 if the download fails). Also debounces the cover search so a slow earlier request can no longer overwrite newer results, drops a dead userId parameter, and reverts an unrelated vite proxy change. --- client/src/components/Trips/TripFormModal.tsx | 8 +++- client/vite.config.js | 20 ++++---- server/src/nest/trips/trips.controller.ts | 18 +++++-- server/src/nest/trips/trips.service.ts | 4 +- server/src/services/placeService.ts | 4 +- server/src/services/unsplashService.ts | 48 ++++++++++++++++++- shared/src/i18n/sv/dashboard.ts | 7 +++ 7 files changed, 89 insertions(+), 20 deletions(-) diff --git a/client/src/components/Trips/TripFormModal.tsx b/client/src/components/Trips/TripFormModal.tsx index b08f9186..88e8de58 100644 --- a/client/src/components/Trips/TripFormModal.tsx +++ b/client/src/components/Trips/TripFormModal.tsx @@ -34,6 +34,7 @@ interface CoverSearchPhoto { export default function TripFormModal({ isOpen, onClose, onSave, trip, onCoverUpdate }: TripFormModalProps) { const isEditing = !!trip const fileRef = useRef(null) + const coverSearchSeq = useRef(0) const toast = useToast() const { t } = useTranslation() const currentUser = useAuthStore(s => s.user) @@ -216,17 +217,22 @@ export default function TripFormModal({ isOpen, onClose, onSave, trip, onCoverUp setCoverSearchError(t('dashboard.unsplashQueryRequired')) return } + // Guard against out-of-order responses: only the latest search applies its + // results, so a slow earlier query can't overwrite a newer one. #1277 review + const seq = ++coverSearchSeq.current setSearchingCover(true) setCoverSearchError('') try { const data = await tripsApi.searchCoverImages(query) + if (seq !== coverSearchSeq.current) return const photos = data.photos || [] setCoverSearchResults(photos) if (photos.length === 0) setCoverSearchError(t('dashboard.unsplashNoResults')) } catch (err: unknown) { + if (seq !== coverSearchSeq.current) return setCoverSearchError(getApiErrorMessage(err, t('dashboard.coverSearchError'))) } finally { - setSearchingCover(false) + if (seq === coverSearchSeq.current) setSearchingCover(false) } } diff --git a/client/vite.config.js b/client/vite.config.js index 1c6d8d4e..35f486b8 100644 --- a/client/vite.config.js +++ b/client/vite.config.js @@ -2,8 +2,6 @@ import { defineConfig } from 'vite' import react from '@vitejs/plugin-react' import { VitePWA } from 'vite-plugin-pwa' -const backendTarget = process.env.VITE_DEV_PROXY_TARGET || 'http://localhost:3001' - export default defineConfig({ plugins: [ react(), @@ -128,42 +126,42 @@ export default defineConfig({ port: 5173, proxy: { '/api': { - target: backendTarget, + target: 'http://localhost:3001', changeOrigin: true, }, '/uploads': { - target: backendTarget, + target: 'http://localhost:3001', changeOrigin: true, }, '/ws': { - target: backendTarget, + target: 'http://localhost:3001', ws: true, }, '/mcp': { - target: backendTarget, + target: 'http://localhost:3001', changeOrigin: true, }, // OAuth 2.1 endpoints handled by backend (SDK authorize handler + token/revoke) // /oauth/authorize goes to backend so the SDK can redirect to /oauth/consent // /oauth/consent is served by Vite as a SPA route (no proxy entry needed) '/oauth/authorize': { - target: backendTarget, + target: 'http://localhost:3001', changeOrigin: true, }, '/oauth/token': { - target: backendTarget, + target: 'http://localhost:3001', changeOrigin: true, }, '/oauth/register': { - target: backendTarget, + target: 'http://localhost:3001', changeOrigin: true, }, '/oauth/revoke': { - target: backendTarget, + target: 'http://localhost:3001', changeOrigin: true, }, '/.well-known': { - target: backendTarget, + target: 'http://localhost:3001', changeOrigin: true, }, } diff --git a/server/src/nest/trips/trips.controller.ts b/server/src/nest/trips/trips.controller.ts index a82ead1d..ca4ecc7d 100644 --- a/server/src/nest/trips/trips.controller.ts +++ b/server/src/nest/trips/trips.controller.ts @@ -29,6 +29,7 @@ import { CurrentUser } from '../auth/current-user.decorator'; import { writeAudit, getClientIp, logInfo } from '../../services/auditLog'; import { isDemoEmail } from '../../services/demo'; import { NotFoundError, ValidationError } from '../../services/tripService'; +import { saveUnsplashCover, isUnsplashCoverUrl } from '../../services/unsplashService'; const MAX_COVER_SIZE = 20 * 1024 * 1024; const coversDir = path.join(__dirname, '../../../uploads/covers'); @@ -72,9 +73,9 @@ export class TripsController { } @Get('cover-images/search') - async coverImages(@CurrentUser() user: User, @Query('query') query?: string) { + async coverImages(@Query('query') query?: string) { try { - const result = await this.trips.searchCoverImages(user.id, query || ''); + const result = await this.trips.searchCoverImages(query || ''); if ('error' in result) { throw new HttpException({ error: result.error }, result.status); } @@ -120,7 +121,7 @@ export class TripsController { } @Put(':id') - update(@CurrentUser() user: User, @Param('id') id: string, @Body() body: Record, @Req() req: Request, @Headers('x-socket-id') socketId?: string) { + async update(@CurrentUser() user: User, @Param('id') id: string, @Body() body: Record, @Req() req: Request, @Headers('x-socket-id') socketId?: string) { const access = this.trips.canAccessTrip(id, user.id); if (!access) { throw new HttpException({ error: 'Trip not found' }, 404); @@ -137,6 +138,17 @@ export class TripsController { if (editFields.some((f) => body[f] !== undefined) && !this.trips.can('trip_edit', user.role, ownerId, user.id, isMember)) { throw new HttpException({ error: 'No permission to edit this trip' }, 403); } + // A chosen Unsplash cover arrives as an images.unsplash.com hot-link; download + // it into uploads/covers so the cover survives offline + CDN link-rot (#1277). + if (isUnsplashCoverUrl(body.cover_image)) { + try { + const filename = await saveUnsplashCover(body.cover_image, coversDir); + body.cover_image = `/uploads/covers/${filename}`; + } catch (e) { + console.error('Unsplash cover download failed:', e); + throw new HttpException({ error: 'Could not save the selected cover image' }, 502); + } + } const oldCover = body.cover_image !== undefined ? (this.trips.getRaw(id) as { cover_image: string | null } | undefined)?.cover_image : undefined; diff --git a/server/src/nest/trips/trips.service.ts b/server/src/nest/trips/trips.service.ts index 34ac2d18..9526e006 100644 --- a/server/src/nest/trips/trips.service.ts +++ b/server/src/nest/trips/trips.service.ts @@ -50,8 +50,8 @@ export class TripsService { return tripSvc.getTripRaw(tripId); } - searchCoverImages(userId: number, query: string) { - return searchUnsplashPhotos(userId, query, 9); + searchCoverImages(query: string) { + return searchUnsplashPhotos(query, 9); } getOwner(tripId: string) { diff --git a/server/src/services/placeService.ts b/server/src/services/placeService.ts index a63b2c1a..4a1e3234 100644 --- a/server/src/services/placeService.ts +++ b/server/src/services/placeService.ts @@ -939,9 +939,9 @@ export async function importNaverList( // Search place image (Unsplash) // --------------------------------------------------------------------------- -export async function searchPlaceImage(tripId: string, placeId: string, userId: number) { +export async function searchPlaceImage(tripId: string, placeId: string, _userId: number) { const place = db.prepare('SELECT * FROM places WHERE id = ? AND trip_id = ?').get(placeId, tripId) as Place | undefined; if (!place) return { error: 'Place not found', status: 404 }; - return searchUnsplashPhotos(userId, place.name + (place.address ? ' ' + place.address : ''), 5); + return searchUnsplashPhotos(place.name + (place.address ? ' ' + place.address : ''), 5); } diff --git a/server/src/services/unsplashService.ts b/server/src/services/unsplashService.ts index 1709a586..c9a6da00 100644 --- a/server/src/services/unsplashService.ts +++ b/server/src/services/unsplashService.ts @@ -1,3 +1,8 @@ +import path from 'path'; +import fs from 'fs'; +import { v4 as uuidv4 } from 'uuid'; +import { safeFetch } from '../utils/ssrfGuard'; + interface UnsplashSearchResponse { results?: { id: string; @@ -20,7 +25,7 @@ export interface UnsplashPhoto { link: string | null; } -export async function searchUnsplashPhotos(_userId: number, query: string, perPage = 9) { +export async function searchUnsplashPhotos(query: string, perPage = 9) { const trimmed = query.trim(); if (!trimmed) { return { error: 'Search query is required', status: 400 }; @@ -67,3 +72,44 @@ export async function searchUnsplashPhotos(_userId: number, query: string, perPa return { photos }; } + +const UNSPLASH_IMAGE_HOST = 'images.unsplash.com'; +const MAX_COVER_BYTES = 15 * 1024 * 1024; +const COVER_EXT_BY_TYPE: Record = { + 'image/jpeg': '.jpg', + 'image/png': '.png', + 'image/webp': '.webp', + 'image/gif': '.gif', +}; + +/** True when a cover_image value is an Unsplash CDN hot-link we should internalise. */ +export function isUnsplashCoverUrl(value: unknown): value is string { + if (typeof value !== 'string') return false; + try { + return new URL(value).hostname.toLowerCase() === UNSPLASH_IMAGE_HOST; + } catch { + return false; + } +} + +/** + * Download a chosen Unsplash cover from its CDN into destDir so the cover is + * stored locally (offline + CDN link-rot safe) instead of hot-linked. Only the + * Unsplash image CDN host is accepted, and the request goes through the SSRF + * guard. Returns the saved filename. Throws on a non-Unsplash host, a failed + * download, an unsupported content type, or an oversized image. + */ +export async function saveUnsplashCover(url: string, destDir: string): Promise { + if (!isUnsplashCoverUrl(url)) throw new Error('Not an Unsplash image URL'); + const res = await safeFetch(url); + if (!res.ok) throw new Error(`Unsplash image download failed (HTTP ${res.status})`); + const type = (res.headers.get('content-type') || '').split(';')[0].trim().toLowerCase(); + const ext = COVER_EXT_BY_TYPE[type]; + if (!ext) throw new Error(`Unsupported cover image type: ${type || 'unknown'}`); + const buf = Buffer.from(await res.arrayBuffer()); + if (buf.byteLength > MAX_COVER_BYTES) throw new Error('Cover image too large'); + if (!fs.existsSync(destDir)) fs.mkdirSync(destDir, { recursive: true }); + const filename = `${uuidv4()}${ext}`; + fs.writeFileSync(path.join(destDir, filename), buf); + return filename; +} diff --git a/shared/src/i18n/sv/dashboard.ts b/shared/src/i18n/sv/dashboard.ts index 42b1834f..70e42689 100644 --- a/shared/src/i18n/sv/dashboard.ts +++ b/shared/src/i18n/sv/dashboard.ts @@ -89,6 +89,13 @@ const dashboard: TranslationStrings = { 'dashboard.coverSaved': 'Omslagsbild sparad', 'dashboard.coverUploadError': 'Det gick inte att ladda upp', 'dashboard.coverRemoveError': 'Det gick inte att ta bort', + 'dashboard.coverSaveError': 'Det gick inte att spara omslagsbilden', + 'dashboard.searchUnsplash': 'Sök på Unsplash', + 'dashboard.unsplashSearchPlaceholder': 'Sök efter resmålsbilder', + 'dashboard.unsplashQueryRequired': 'Ange en sökterm', + 'dashboard.unsplashNoResults': 'Inga bilder hittades', + 'dashboard.coverSearchError': 'Det gick inte att söka på Unsplash', + 'dashboard.useUnsplashPhoto': 'Använd Unsplash-foto av {photographer}', 'dashboard.titleRequired': 'Titel är obligatoriskt', 'dashboard.endDateError': 'Slutdatumet måste ligga efter startdatumet', 'dashboard.greeting.morning': 'God morgon,',