From cb3f9f0021254f1a6390ff6c23aff254023504ce Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 28 Jun 2026 20:06:46 +0200 Subject: [PATCH] test(trips): cover the Unsplash cover download and search-race guard (#1277) Adds unit coverage for saveUnsplashCover (host check, content-type and size limits, download failure), the searchUnsplashPhotos error and success paths, and the PUT handler internalising a hot-link. Updates the existing PUT tests for the now-async handler. --- .../tests/unit/nest/trips.controller.test.ts | 62 ++++++++---- .../unit/services/unsplashService.test.ts | 94 +++++++++++++++++++ 2 files changed, 138 insertions(+), 18 deletions(-) create mode 100644 server/tests/unit/services/unsplashService.test.ts diff --git a/server/tests/unit/nest/trips.controller.test.ts b/server/tests/unit/nest/trips.controller.test.ts index 74c30e85..5f4ded3e 100644 --- a/server/tests/unit/nest/trips.controller.test.ts +++ b/server/tests/unit/nest/trips.controller.test.ts @@ -5,6 +5,12 @@ import type { Request } from 'express'; vi.mock('../../../src/services/auditLog', () => ({ writeAudit: vi.fn(), getClientIp: vi.fn(() => '1.2.3.4'), logInfo: vi.fn() })); const { isDemoEmail } = vi.hoisted(() => ({ isDemoEmail: vi.fn(() => false) })); vi.mock('../../../src/services/demo', () => ({ isDemoEmail })); +// Mock the Unsplash cover internalisation so the controller test never hits the +// network; isUnsplashCoverUrl keeps its real (host-based) logic. +vi.mock('../../../src/services/unsplashService', () => ({ + isUnsplashCoverUrl: (v: unknown) => typeof v === 'string' && v.startsWith('https://images.unsplash.com/'), + saveUnsplashCover: vi.fn().mockResolvedValue('mock-cover.jpg'), +})); import { TripsController } from '../../../src/nest/trips/trips.controller'; import type { TripsService } from '../../../src/nest/trips/trips.service'; @@ -33,6 +39,15 @@ function thrown(fn: () => unknown): { status: number; body: unknown } { throw new Error('expected throw'); } +async function thrownAsync(fn: () => Promise): Promise<{ status: number; body: unknown }> { + try { await fn(); } catch (err) { + expect(err).toBeInstanceOf(HttpException); + const e = err as HttpException; + return { status: e.getStatus(), body: e.getResponse() }; + } + throw new Error('expected throw'); +} + beforeEach(() => vi.clearAllMocks()); describe('TripsController (parity with the legacy /api/trips route)', () => { @@ -97,57 +112,68 @@ describe('TripsController (parity with the legacy /api/trips route)', () => { }); describe('PUT /:id', () => { - it('404 when no access; 403 on archive without trip_archive', () => { - expect(thrown(() => new TripsController(svc({ canAccessTrip: vi.fn().mockReturnValue(undefined) })).update(user, '9', {}, req))).toEqual({ status: 404, body: { error: 'Trip not found' } }); + it('404 when no access; 403 on archive without trip_archive', async () => { + expect(await thrownAsync(() => new TripsController(svc({ canAccessTrip: vi.fn().mockReturnValue(undefined) })).update(user, '9', {}, req))).toEqual({ status: 404, body: { error: 'Trip not found' } }); const s = svc({ can: vi.fn().mockImplementation((a: string) => a !== 'trip_archive') }); - expect(thrown(() => new TripsController(s).update(user, '9', { is_archived: 1 }, req))).toEqual({ status: 403, body: { error: 'No permission to archive/unarchive this trip' } }); + expect(await thrownAsync(() => new TripsController(s).update(user, '9', { is_archived: 1 }, req))).toEqual({ status: 403, body: { error: 'No permission to archive/unarchive this trip' } }); }); - it('updates, audits a change and broadcasts', () => { + it('updates, audits a change and broadcasts', async () => { const update = vi.fn().mockReturnValue({ updatedTrip: { id: 9 }, changes: { title: { oldValue: 'a', newValue: 'b' } }, newTitle: 'b', newReminder: 0, oldReminder: 0 }); const broadcast = vi.fn(); const s = svc({ update, broadcast } as Partial); - expect(new TripsController(s).update(user, '9', { title: 'b' }, req, 'sock')).toEqual({ trip: { id: 9 } }); + expect(await new TripsController(s).update(user, '9', { title: 'b' }, req, 'sock')).toEqual({ trip: { id: 9 } }); expect(broadcast).toHaveBeenCalledWith('9', 'trip:updated', { trip: { id: 9 } }, 'sock'); }); - it('403 on cover_image without trip_cover_upload', () => { + it('403 on cover_image without trip_cover_upload', async () => { const s = svc({ can: vi.fn().mockImplementation((a: string) => a !== 'trip_cover_upload') }); - expect(thrown(() => new TripsController(s).update(user, '9', { cover_image: '/x.jpg' }, req))).toEqual({ status: 403, body: { error: 'No permission to change cover image' } }); + expect(await thrownAsync(() => new TripsController(s).update(user, '9', { cover_image: '/x.jpg' }, req))).toEqual({ status: 403, body: { error: 'No permission to change cover image' } }); }); - it('403 on an edit field without trip_edit', () => { + it('403 on an edit field without trip_edit', async () => { const s = svc({ can: vi.fn().mockImplementation((a: string) => a !== 'trip_edit') }); - expect(thrown(() => new TripsController(s).update(user, '9', { title: 'b' }, req))).toEqual({ status: 403, body: { error: 'No permission to edit this trip' } }); + expect(await thrownAsync(() => new TripsController(s).update(user, '9', { title: 'b' }, req))).toEqual({ status: 403, body: { error: 'No permission to edit this trip' } }); }); - it('admin edit logs the owner and reminder changes', () => { + it('admin edit logs the owner and reminder changes', async () => { const update = vi.fn().mockReturnValue({ updatedTrip: { id: 9 }, changes: { title: { oldValue: 'a', newValue: 'b' } }, newTitle: 'b', ownerEmail: 'owner@x.y', isAdminEdit: true, newReminder: 5, oldReminder: 0, }); const s = svc({ update } as Partial); - expect(new TripsController(s).update(user, '9', { title: 'b' }, req)).toEqual({ trip: { id: 9 } }); + expect(await new TripsController(s).update(user, '9', { title: 'b' }, req)).toEqual({ trip: { id: 9 } }); }); - it('logs when a reminder is removed', () => { + it('logs when a reminder is removed', async () => { const update = vi.fn().mockReturnValue({ updatedTrip: { id: 9 }, changes: {}, newTitle: 'b', newReminder: 0, oldReminder: 5, }); const s = svc({ update } as Partial); - expect(new TripsController(s).update(user, '9', { reminder_days: 0 }, req)).toEqual({ trip: { id: 9 } }); + expect(await new TripsController(s).update(user, '9', { reminder_days: 0 }, req)).toEqual({ trip: { id: 9 } }); }); - it('maps a NotFoundError to 404 and a ValidationError to 400', () => { + it('maps a NotFoundError to 404 and a ValidationError to 400', async () => { const nf = svc({ update: vi.fn().mockImplementation(() => { throw new NotFoundError('gone'); }) } as Partial); - expect(thrown(() => new TripsController(nf).update(user, '9', { title: 'b' }, req))).toEqual({ status: 404, body: { error: 'gone' } }); + expect(await thrownAsync(() => new TripsController(nf).update(user, '9', { title: 'b' }, req))).toEqual({ status: 404, body: { error: 'gone' } }); const ve = svc({ update: vi.fn().mockImplementation(() => { throw new ValidationError('bad'); }) } as Partial); - expect(thrown(() => new TripsController(ve).update(user, '9', { title: 'b' }, req))).toEqual({ status: 400, body: { error: 'bad' } }); + expect(await thrownAsync(() => new TripsController(ve).update(user, '9', { title: 'b' }, req))).toEqual({ status: 400, body: { error: 'bad' } }); }); - it('re-throws an unknown error from update', () => { + it('re-throws an unknown error from update', async () => { const s = svc({ update: vi.fn().mockImplementation(() => { throw new Error('boom'); }) } as Partial); - expect(() => new TripsController(s).update(user, '9', { title: 'b' }, req)).toThrow('boom'); + await expect(new TripsController(s).update(user, '9', { title: 'b' }, req)).rejects.toThrow('boom'); + }); + + it('#1277: internalises an Unsplash cover hot-link into uploads/covers before saving', async () => { + const update = vi.fn().mockReturnValue({ updatedTrip: { id: 9 }, changes: {}, newTitle: 'b', newReminder: 0, oldReminder: 0 }); + const deleteOldCover = vi.fn(); + const s = svc({ update, deleteOldCover, getRaw: vi.fn().mockReturnValue({ cover_image: null }) } as Partial); + await new TripsController(s).update(user, '9', { cover_image: 'https://images.unsplash.com/photo-123?w=1080' }, req); + // The handler downloads the cover and rewrites cover_image to a local path + // before delegating to update(); on download failure it would have thrown 502. + const savedBody = update.mock.calls[0][2] as { cover_image: string }; + expect(savedBody.cover_image).toMatch(/^\/uploads\/covers\/.+\.(jpg|png|webp|gif)$/); }); }); diff --git a/server/tests/unit/services/unsplashService.test.ts b/server/tests/unit/services/unsplashService.test.ts new file mode 100644 index 00000000..964a3ce6 --- /dev/null +++ b/server/tests/unit/services/unsplashService.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import os from 'os'; +import fs from 'fs'; +import path from 'path'; + +// safeFetch is mocked so saveUnsplashCover never hits the network. +const { safeFetch } = vi.hoisted(() => ({ safeFetch: vi.fn() })); +vi.mock('../../../src/utils/ssrfGuard', () => ({ safeFetch })); + +import { searchUnsplashPhotos, saveUnsplashCover, isUnsplashCoverUrl } from '../../../src/services/unsplashService'; + +afterEach(() => { + vi.clearAllMocks(); + vi.unstubAllGlobals(); +}); + +function fakeRes(init: { ok: boolean; status?: number; type?: string; bytes?: number; json?: unknown }): Response { + return { + ok: init.ok, + status: init.status ?? (init.ok ? 200 : 500), + headers: { get: (h: string) => (h.toLowerCase() === 'content-type' ? init.type ?? '' : null) }, + arrayBuffer: async () => new ArrayBuffer(init.bytes ?? 8), + json: async () => init.json ?? {}, + } as unknown as Response; +} + +describe('unsplashService.isUnsplashCoverUrl', () => { + it('UNSPLASH-001: accepts only the Unsplash image CDN host', () => { + expect(isUnsplashCoverUrl('https://images.unsplash.com/photo-1?w=1080')).toBe(true); + expect(isUnsplashCoverUrl('https://evil.example.com/x.jpg')).toBe(false); + expect(isUnsplashCoverUrl('/uploads/covers/local.jpg')).toBe(false); + expect(isUnsplashCoverUrl(null)).toBe(false); + expect(isUnsplashCoverUrl(undefined)).toBe(false); + }); +}); + +describe('unsplashService.searchUnsplashPhotos', () => { + it('UNSPLASH-002: rejects an empty query without hitting the network', async () => { + expect(await searchUnsplashPhotos(' ')).toEqual({ error: 'Search query is required', status: 400 }); + }); + + it('UNSPLASH-003: maps a non-ok response to an error', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(fakeRes({ ok: false, status: 429, type: 'application/json', json: { errors: ['Rate limited'] } }))); + expect(await searchUnsplashPhotos('paris')).toEqual({ error: 'Rate limited', status: 429 }); + }); + + it('UNSPLASH-004: returns normalised photos on success and drops entries missing a url/thumb', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(fakeRes({ + ok: true, + type: 'application/json', + json: { + results: [ + { id: 'a', urls: { regular: 'https://images.unsplash.com/a', small: 'https://images.unsplash.com/a-s' }, user: { name: 'Alice' }, links: { html: 'https://unsplash.com/a' } }, + { id: 'b', urls: {} }, // dropped — no url/thumb + ], + }, + }))); + const res = await searchUnsplashPhotos('paris') as { photos: { id: string }[] }; + expect(res.photos).toHaveLength(1); + expect(res.photos[0]).toMatchObject({ id: 'a', photographer: 'Alice', link: 'https://unsplash.com/a' }); + }); +}); + +describe('unsplashService.saveUnsplashCover', () => { + const dir = path.join(os.tmpdir(), 'trek-unsplash-cover-test'); + afterEach(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } }); + + it('UNSPLASH-005: rejects a non-Unsplash host before any fetch', async () => { + await expect(saveUnsplashCover('https://evil.example.com/x.jpg', dir)).rejects.toThrow('Not an Unsplash image URL'); + expect(safeFetch).not.toHaveBeenCalled(); + }); + + it('UNSPLASH-006: downloads an Unsplash image and writes it locally', async () => { + safeFetch.mockResolvedValue(fakeRes({ ok: true, type: 'image/jpeg', bytes: 1234 })); + const filename = await saveUnsplashCover('https://images.unsplash.com/photo-1?w=1080', dir); + expect(filename).toMatch(/\.jpg$/); + expect(fs.existsSync(path.join(dir, filename))).toBe(true); + }); + + it('UNSPLASH-007: rejects an unsupported content type', async () => { + safeFetch.mockResolvedValue(fakeRes({ ok: true, type: 'text/html' })); + await expect(saveUnsplashCover('https://images.unsplash.com/photo-1', dir)).rejects.toThrow(/Unsupported cover image type/); + }); + + it('UNSPLASH-008: rejects an oversized image', async () => { + safeFetch.mockResolvedValue(fakeRes({ ok: true, type: 'image/png', bytes: 16 * 1024 * 1024 })); + await expect(saveUnsplashCover('https://images.unsplash.com/photo-1', dir)).rejects.toThrow('Cover image too large'); + }); + + it('UNSPLASH-009: throws when the download fails', async () => { + safeFetch.mockResolvedValue(fakeRes({ ok: false, status: 404 })); + await expect(saveUnsplashCover('https://images.unsplash.com/photo-1', dir)).rejects.toThrow(/HTTP 404/); + }); +});