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.

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<unknown>): 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<TripsService>);
-      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<TripsService>);
-      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<TripsService>);
-      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<TripsService>);
-      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<TripsService>);
-      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<TripsService>);
-      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<TripsService>);
+      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)$/);
     });
   });
 

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/);
+  });
+});