k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-20 14:06:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(trips): trip covers through storage

Browse Source
This commit is contained in:
jubnl
2026-08-17 05:37:52 +02:00
parent 965566369e
commit 8a3327c5f6
11 changed files with 87 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/nest/trips/trips.controller.ts
+3 -4
View File
@@ -40,7 +40,6 @@ import { TripReadModelService } from '../trip-read-model/trip-read-model.service
export const MAX_COVER_SIZE = 20 * 1024 * 1024;
// Still needed by the Unsplash cover download (a raw-fs writer until the
// caches/downloads slice); the multer destination moved to the storage spool.
const coversDir = path.join(__dirname, '../../../uploads/covers');
// Consumed by trips.module.ts's MulterModule factory. Quirk preserved on
// purpose: a plain Error without statusCode maps to 500, not 400 (parity).
export const TRIP_COVER_FILE_FILTER: Options['fileFilter'] = (_req, file, cb) => {
@@ -154,7 +153,7 @@ export class TripsController {
// it into uploads/covers so the cover survives offline + CDN link-rot (#1277).
if (this.unsplash.isUnsplashCoverUrl(body.cover_image)) {
try {
const filename = await this.unsplash.saveUnsplashCover(body.cover_image, coversDir);
const filename = await this.unsplash.saveUnsplashCover(body.cover_image);
body.cover_image = `/uploads/covers/${filename}`;
} catch (e) {
console.error('Unsplash cover download failed:', e);
@@ -167,7 +166,7 @@ export class TripsController {
try {
const result = await this.trips.update(id, user.id, body, user.role);
if (body.cover_image !== undefined && body.cover_image !== oldCover) {
this.trips.deleteOldCover(oldCover);
await this.trips.deleteOldCover(oldCover);
}
if (Object.keys(result.changes).length > 0) {
this.audit.writeAudit({ userId: user.id, action: 'trip.update', ip: getClientIp(req), details: { tripId: Number(id), trip: result.newTitle, ...(result.ownerEmail ? { owner: result.ownerEmail } : {}), ...result.changes } });
@@ -209,7 +208,7 @@ export class TripsController {
// Commit the spooled upload to its final storage location (atomic
// same-volume rename) before anything references the final path.
await this.storage.put('covers', file.filename, { tmpPath: file.path });
this.trips.deleteOldCover(trip.cover_image);
await this.trips.deleteOldCover(trip.cover_image);
const coverUrl = `/uploads/covers/${file.filename}`;
this.trips.updateCoverImage(id, coverUrl);
return { cover_image: coverUrl };
server/src/nest/trips/trips.service.ts
+10 -11
View File
@@ -1,6 +1,5 @@
import { Injectable } from '@nestjs/common';
import path from 'path';
import fs from 'fs';
import { DatabaseService } from '../database/database.service';
import type { ActiveTrip, TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
import { RealtimeService } from '../realtime/realtime.service';
@@ -11,6 +10,7 @@ import { BudgetService } from '../budget/budget.service';
import { ReservationsService } from '../reservations/reservations.service';
import { VacayService } from '../vacay/vacay.service';
import { UnsplashService } from '../unsplash/unsplash.service';
import { StorageService } from '../storage/storage.service';
import { NotFoundError, ValidationError } from '../common/domain-errors';
export const MS_PER_DAY = 86400000;
@@ -146,6 +146,7 @@ export class TripsService {
private readonly vacay: VacayService,
private readonly realtime: RealtimeService,
private readonly unsplash: UnsplashService,
private readonly storage: StorageService,
) {}
private get db() {
@@ -499,17 +500,15 @@ export class TripsService {
// ── Cover image ───────────────────────────────────────────────────────────
deleteOldCover(coverImage: string | null | undefined): void {
async deleteOldCover(coverImage: string | null | undefined): Promise<void> {
if (!coverImage) return;
// cover_image is client-supplied, so treat it as untrusted: covers live in
// uploads/covers as a flat filename — use basename() and confine the unlink
// to that directory. (dist/nest/trips is one level deeper than the legacy
// dist/services home, hence the extra '..' — same resolved directory.)
const coversDir = path.resolve(__dirname, '../../../uploads/covers');
const resolvedPath = path.resolve(path.join(coversDir, path.basename(coverImage)));
if (resolvedPath.startsWith(coversDir + path.sep) && fs.existsSync(resolvedPath)) {
fs.unlinkSync(resolvedPath);
}
// cover_image is client-supplied, so treat it as untrusted: covers are flat
// filenames in the 'covers' category — basename() confines the delete to
// it, and central key validation rejects anything hostile (swallowed like
// the old containment guard; an external https URL is likewise a no-op).
await this.storage.delete('covers', path.basename(coverImage)).catch(() => {
/* external URL or already gone */
});
}
updateCoverImage(tripId: string | number, coverUrl: string): void {
server/src/nest/unsplash/unsplash.module.ts
+2 -1
View File
@@ -1,6 +1,7 @@
import { Module } from '@nestjs/common';
import { UnsplashService } from './unsplash.service';
import { AppConfigModule } from '../app-config/app-config.module';
import { StorageModule } from '../storage/storage.module';
/** Unsplash cover search and download. No controller of its own — trips and
* places both reach it. Deliberately NOT @Global, matching PermissionsModule:
@@ -8,7 +9,7 @@ import { AppConfigModule } from '../app-config/app-config.module';
* AppConfigModule is imported explicitly because @Global only applies to
* modules that are in the graph, which a single-domain TestingModule is not. */
@Module({
imports: [AppConfigModule],
imports: [AppConfigModule, StorageModule],
providers: [UnsplashService],
exports: [UnsplashService],
})
server/src/nest/unsplash/unsplash.service.ts
+11 -10
View File
@@ -1,11 +1,11 @@
import path from 'path';
import fs from 'fs';
import { Readable } from 'node:stream';
import { v4 as uuidv4 } from 'uuid';
import { Injectable } from '@nestjs/common';
import { safeFetch } from '../../utils/ssrfGuard';
import { decrypt_api_key } from '../common/crypto/apiKeyCrypto';
import { DatabaseService } from '../database/database.service';
import { RuntimeEnvService } from '../app-config/runtime-env.service';
import { StorageService } from '../storage/storage.service';
interface UnsplashSearchResponse {
results?: {
@@ -50,6 +50,7 @@ export class UnsplashService {
constructor(
private readonly db: DatabaseService,
private readonly env: RuntimeEnvService,
private readonly storage: StorageService,
) {}
/**
@@ -138,13 +139,14 @@ export class UnsplashService {
}
/**
* 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.
* Download a chosen Unsplash cover from its CDN into the 'covers' storage
* category 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 — validation runs before any write.
*/
async saveUnsplashCover(url: string, destDir: string): Promise<string> {
async saveUnsplashCover(url: string): Promise<string> {
if (!this.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})`);
@@ -153,9 +155,8 @@ export class UnsplashService {
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);
await this.storage.put('covers', filename, Readable.from(buf));
return filename;
}
}
server/tests/helpers/mcp-test-controllers.ts
+3 -2
View File
@@ -123,7 +123,7 @@ export function createMcpTestRegistry(): McpRegistry {
// includes `src`, so nothing here is typechecked; pass them for real.
const placesService = new PlacesService(
dbService, permissionsService, realtimeService, mapsService, queryHelpersService,
new UnsplashService(dbService, new RuntimeEnvService()),
new UnsplashService(dbService, new RuntimeEnvService(), generalStorage),
placePhotoCache,
journeyDomain,
generalStorage,
@@ -139,7 +139,8 @@ export function createMcpTestRegistry(): McpRegistry {
budgetService,
new VacayService(dbService, realtimeService, notificationsStub()),
realtimeService,
new UnsplashService(dbService, new RuntimeEnvService()),
new UnsplashService(dbService, new RuntimeEnvService(), generalStorage),
generalStorage,
);
const readModelService = new TripReadModelService(
dbService, membersService, daysService, accommodationsService, budgetService,
server/tests/helpers/plugin-host.ts
+2 -2
View File
@@ -98,7 +98,7 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF
const days = new DaysService(dbs, permissions, realtime, queryHelpers);
const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/google/').storage);
const generalStorage = makeStorageFixture('').storage;
const unsplash = new UnsplashService(dbs, new RuntimeEnvService());
const unsplash = new UnsplashService(dbs, new RuntimeEnvService(), generalStorage);
const journey = new JourneyDomainService(dbs, realtime, new TrekPhotosRepository(dbs));
const places = new PlacesService(dbs, permissions, realtime, new MapsService(dbs, photoCache), queryHelpers, unsplash, photoCache, journey, generalStorage);
const collections = new CollectionsService(dbs, permissions, realtime, notificationsStub(), generalStorage);
@@ -110,7 +110,7 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF
const llmConfig = new LlmConfigResolver(new SettingsService(dbs), dbs, addons);
const oauth = new PluginOAuthService(dbs);
const accommodations = new AccommodationsService(dbs, permissions, realtime);
const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash);
const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash, generalStorage);
const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget), permissions, realtime, notificationsStub());
const guards = new PluginGuards(dbs, permissions, addons);
server/tests/unit/nest/places.service.test.ts
+1 -1
View File
@@ -100,7 +100,7 @@ function makePlacesService(maps: MapsService = new MapsService(dbs, photoCacheSt
new RealtimeService(),
maps,
new QueryHelpersService(dbs),
new UnsplashService(dbs, new RuntimeEnvService()),
new UnsplashService(dbs, new RuntimeEnvService(), placesStorageFx.storage),
photoCacheStub,
new JourneyDomainService(dbs, new RealtimeService(), new TrekPhotosRepository(dbs)),
placesStorageFx.storage,
server/tests/unit/nest/trip-read-model.service.test.ts
+1 -1
View File
@@ -88,7 +88,7 @@ const photoCache = new PlacePhotoCacheService(dbs(), makeStorageFixture('photos/
const placesSvc = new PlacesService(
dbs(), new PermissionsService(dbs()), new RealtimeService(),
new MapsService(dbs(), photoCache), new QueryHelpersService(dbs()),
new UnsplashService(dbs(), new RuntimeEnvService()), photoCache,
new UnsplashService(dbs(), new RuntimeEnvService(), makeStorageFixture('').storage), photoCache,
new JourneyDomainService(dbs(), new RealtimeService(), new TrekPhotosRepository(dbs())),
makeStorageFixture('').storage,
);
server/tests/unit/nest/trips.service.test.ts
+35 -30
View File
@@ -82,6 +82,7 @@ import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.serv
import { makeStorageFixture } from '../../helpers/storage-fixture';
import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service';
import fs from 'fs';
import path from 'path';
import { notificationsStub } from '../../helpers/notifications';
import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service';
@@ -98,13 +99,14 @@ const daysSvc = new DaysService(dbs(), new PermissionsService(dbs()), new Realti
// One PlacePhotoCacheService for both PlacesService and MapsService, matching
// production, where the in-flight dedup only works on a shared instance.
const photoCache = new PlacePhotoCacheService(dbs(), makeStorageFixture('photos/google/').storage);
const coversFx = makeStorageFixture('covers/');
const placesSvc = new PlacesService(
dbs(),
new PermissionsService(dbs()),
new RealtimeService(),
new MapsService(dbs(), photoCache),
new QueryHelpersService(dbs()),
new UnsplashService(dbs(), new RuntimeEnvService()),
new UnsplashService(dbs(), new RuntimeEnvService(), coversFx.storage),
photoCache,
new JourneyDomainService(dbs(), new RealtimeService(), new TrekPhotosRepository(dbs())),
makeStorageFixture('').storage,
@@ -121,6 +123,7 @@ const svc = new TripsService(
new VacayService(dbs(), new RealtimeService(), notificationsStub()),
new RealtimeService(),
undefined as never, // unsplash — not exercised here
coversFx.storage,
);
const membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc), new PermissionsService(dbs()), new RealtimeService(), notificationsStub());
const readModelSvc = new TripReadModelService(
@@ -372,38 +375,39 @@ describe('generateDays', () => {
// ── deleteOldCover — path containment ──────────────────────────────────────────
describe('deleteOldCover', () => {
it('TRIP-SVC-COVER-001: never unlinks outside uploads/covers for a crafted cover_image', () => {
const existsSpy = vi.spyOn(fs, 'existsSync').mockReturnValue(true);
const unlinkSpy = vi.spyOn(fs, 'unlinkSync').mockImplementation(() => {});
try {
// Attacker-controlled values aimed at auth-gated sibling upload dirs.
svc.deleteOldCover('/uploads/files/victim.pdf');
svc.deleteOldCover('/uploads/covers/../files/secret.pdf');
svc.deleteOldCover('/uploads/avatars/someone.png');
it('TRIP-SVC-COVER-001: never deletes outside the covers category for a crafted cover_image', async () => {
// Attacker-controlled values aimed at auth-gated sibling upload dirs — the
// basename + category addressing keeps every delete inside covers/.
const filesDir = path.join(coversFx.root, 'files');
const avatarsDir = path.join(coversFx.root, 'avatars');
fs.mkdirSync(filesDir, { recursive: true });
fs.mkdirSync(avatarsDir, { recursive: true });
const secret = path.join(filesDir, 'secret.pdf');
const someone = path.join(avatarsDir, 'someone.png');
fs.writeFileSync(secret, 'pdf');
fs.writeFileSync(someone, 'png');
for (const call of unlinkSpy.mock.calls) {
const target = String(call[0]);
expect(target).toMatch(/[\\/]uploads[\\/]covers[\\/]/); // stays in covers
expect(target).not.toMatch(/[\\/]files[\\/]/);
expect(target).not.toMatch(/[\\/]avatars[\\/]/);
}
} finally {
existsSpy.mockRestore();
unlinkSpy.mockRestore();
}
await svc.deleteOldCover('/uploads/files/secret.pdf');
await svc.deleteOldCover('/uploads/covers/../files/secret.pdf');
await svc.deleteOldCover('/uploads/avatars/someone.png');
expect(fs.existsSync(secret)).toBe(true);
expect(fs.existsSync(someone)).toBe(true);
});
it('TRIP-SVC-COVER-002: deletes a legitimate cover file', () => {
const existsSpy = vi.spyOn(fs, 'existsSync').mockReturnValue(true);
const unlinkSpy = vi.spyOn(fs, 'unlinkSync').mockImplementation(() => {});
try {
svc.deleteOldCover('/uploads/covers/abc123.jpg');
expect(unlinkSpy).toHaveBeenCalledTimes(1);
expect(String(unlinkSpy.mock.calls[0][0])).toMatch(/[\\/]covers[\\/]abc123\.jpg$/);
} finally {
existsSpy.mockRestore();
unlinkSpy.mockRestore();
}
it('TRIP-SVC-COVER-002: deletes a legitimate cover file', async () => {
const coversDir = path.join(coversFx.root, 'covers');
fs.mkdirSync(coversDir, { recursive: true });
const cover = path.join(coversDir, 'abc123.jpg');
fs.writeFileSync(cover, 'jpeg');
await svc.deleteOldCover('/uploads/covers/abc123.jpg');
expect(fs.existsSync(cover)).toBe(false);
});
it('TRIP-SVC-COVER-003: an external https cover URL is tolerated (no throw)', async () => {
await expect(svc.deleteOldCover('https://example.com/some/pic.jpg')).resolves.toBeUndefined();
await expect(svc.deleteOldCover(null)).resolves.toBeUndefined();
});
});
@@ -1017,6 +1021,7 @@ describe('quirk fixes', () => {
new VacayService(dbs(), new RealtimeService(), notificationsStub()),
new RealtimeService(),
undefined as never,
coversFx.storage,
);
}
server/tests/unit/nest/unsplash/unsplash.service.test.ts
+18 -13
View File
@@ -1,5 +1,4 @@
import { describe, it, expect, vi, afterEach } from 'vitest';
import os from 'os';
import fs from 'fs';
import path from 'path';
@@ -17,9 +16,11 @@ import { UnsplashService } from '../../../../src/nest/unsplash/unsplash.service'
import { DatabaseService } from '../../../../src/nest/database/database.service';
import { RuntimeEnvService } from '../../../../src/nest/app-config/runtime-env.service';
import { db } from '../../../../src/db/database';
import { makeStorageFixture } from '../../../helpers/storage-fixture';
// Same four entry points, now methods. The db mock above still feeds them.
const svc = new UnsplashService(new DatabaseService(db), new RuntimeEnvService());
const coverFx = makeStorageFixture('covers/');
const svc = new UnsplashService(new DatabaseService(db), new RuntimeEnvService(), coverFx.storage);
const searchUnsplashPhotos = svc.searchUnsplashPhotos.bind(svc);
const getUnsplashKey = svc.getUnsplashKey.bind(svc);
const saveUnsplashCover = svc.saveUnsplashCover.bind(svc);
@@ -130,33 +131,37 @@ describe('unsplashService.getUnsplashKey', () => {
});
describe('unsplashService.saveUnsplashCover', () => {
const dir = path.join(os.tmpdir(), 'trek-unsplash-cover-test');
afterEach(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } });
const coversDir = path.join(coverFx.root, 'covers');
const writtenCovers = () => (fs.existsSync(coversDir) ? fs.readdirSync(coversDir) : []);
afterEach(() => { try { fs.rmSync(coversDir, { 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');
await expect(saveUnsplashCover('https://evil.example.com/x.jpg')).rejects.toThrow('Not an Unsplash image URL');
expect(safeFetch).not.toHaveBeenCalled();
});
it('UNSPLASH-006: downloads an Unsplash image and writes it locally', async () => {
it('UNSPLASH-006: downloads an Unsplash image and stores it under covers', async () => {
safeFetch.mockResolvedValue(fakeRes({ ok: true, type: 'image/jpeg', bytes: 1234 }));
const filename = await saveUnsplashCover('https://images.unsplash.com/photo-1?w=1080', dir);
const filename = await saveUnsplashCover('https://images.unsplash.com/photo-1?w=1080');
expect(filename).toMatch(/\.jpg$/);
expect(fs.existsSync(path.join(dir, filename))).toBe(true);
expect(fs.existsSync(path.join(coversDir, filename))).toBe(true);
});
it('UNSPLASH-007: rejects an unsupported content type', async () => {
it('UNSPLASH-007: rejects an unsupported content type without writing', 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/);
await expect(saveUnsplashCover('https://images.unsplash.com/photo-1')).rejects.toThrow(/Unsupported cover image type/);
expect(writtenCovers()).toEqual([]);
});
it('UNSPLASH-008: rejects an oversized image', async () => {
it('UNSPLASH-008: rejects an oversized image without writing', 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');
await expect(saveUnsplashCover('https://images.unsplash.com/photo-1')).rejects.toThrow('Cover image too large');
expect(writtenCovers()).toEqual([]);
});
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/);
await expect(saveUnsplashCover('https://images.unsplash.com/photo-1')).rejects.toThrow(/HTTP 404/);
expect(writtenCovers()).toEqual([]);
});
});
server/tests/unit/services/conflictUpdate.test.ts
+1 -1
View File
@@ -68,7 +68,7 @@ const places = new PlacesService(
realtime,
new MapsService(dbs, photoCache),
new QueryHelpersService(dbs),
new UnsplashService(dbs, runtimeEnv),
new UnsplashService(dbs, runtimeEnv, makeStorageFixture('').storage),
photoCache,
new JourneyDomainService(dbs, realtime, new TrekPhotosRepository(dbs)),
makeStorageFixture('').storage,