diff --git a/server/src/nest/collab/collab.controller.ts b/server/src/nest/collab/collab.controller.ts index 60d39a710..775e1c489 100644 --- a/server/src/nest/collab/collab.controller.ts +++ b/server/src/nest/collab/collab.controller.ts @@ -23,7 +23,7 @@ import type { User } from '../../types'; import { CollabService } from './collab.service'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { BLOCKED_EXTENSIONS } from '../../services/fileService'; +import { BLOCKED_EXTENSIONS } from '../files/files.constants'; const MAX_NOTE_FILE_SIZE = 50 * 1024 * 1024; const filesDir = path.join(__dirname, '../../../uploads/files'); diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts index f28efaf81..b972dbe78 100644 --- a/server/src/nest/common/body-contract-allow-list.ts +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -74,9 +74,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ 'DaysController.reorder', 'DaysController.transport', 'DaysController.update', - 'FilesController.link', - 'FilesController.update', - 'FilesController.upload', 'ImmichMemoriesController.putSettings', 'ImmichMemoriesController.search', 'ImmichMemoriesController.test', diff --git a/server/src/nest/files/files.bridge.ts b/server/src/nest/files/files.bridge.ts new file mode 100644 index 000000000..18a79c83e --- /dev/null +++ b/server/src/nest/files/files.bridge.ts @@ -0,0 +1,21 @@ +import { db } from '../../db/database'; +import { DatabaseService } from '../database/database.service'; +import { FilesService } from './files.service'; + +/** + * Non-Nest entry point for the files domain. Unlike todo.bridge.ts this does + * NOT serve a legacy MCP registrar (files has no MCP surface) — it exists only + * because multer interceptor options are module-scope object literals (the + * UPLOAD configs in files.controller.ts and journey.controller.ts are built + * before any DI container exists), yet their fileFilter closures need the + * request-time app_settings read. Everything else injects FilesService. + * Delete this file if uploads ever move to MulterModule.registerAsync. + * + * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto + * the shared better-sqlite3 singleton. + */ +const files = new FilesService(new DatabaseService(db)); + +export function getAllowedExtensions(): string { + return files.getAllowedExtensions(); +} diff --git a/server/src/nest/files/files.constants.ts b/server/src/nest/files/files.constants.ts new file mode 100644 index 000000000..e3144ce1b --- /dev/null +++ b/server/src/nest/files/files.constants.ts @@ -0,0 +1,44 @@ +import path from 'path'; + +/** + * File-domain constants and pure helpers, split out of FilesService because + * they are consumed at module-load time (multer interceptor configs in the + * files/collab/journey controllers and the plugin host factory) where no DI + * container exists yet. Everything here is DB-free and side-effect-free at + * import; the one request-time DB read those configs need lives in + * files.bridge.ts. + */ + +export const MAX_FILE_SIZE = 50 * 1024 * 1024; // 50 MB +export const DEFAULT_ALLOWED_EXTENSIONS = 'jpg,jpeg,png,gif,webp,heic,pdf,doc,docx,xls,xlsx,txt,csv,pkpass,pkpasses,md,markdown'; + +// Video support (#823). Gallery/media uploads accept these in addition to images, +// independent of the admin doc-types allowlist. Videos are stored as-is and +// streamed with HTTP Range; the cap is higher than images because phone clips are +// large. +export const VIDEO_EXTENSIONS = ['mp4', 'm4v', 'webm', 'mov']; +export const MAX_VIDEO_SIZE = 500 * 1024 * 1024; // 500 MB + +export function isVideoMime(mime: string | undefined | null): boolean { + return !!mime && mime.startsWith('video/'); +} + +export function isVideoExtension(ext: string): boolean { + return VIDEO_EXTENSIONS.includes(ext.toLowerCase().replace(/^\./, '')); +} +// Single authoritative blocklist for every file-upload surface (main +// file manager + collab attachments). When the admin setting +// `allowed_file_types` is `*`, this list is still enforced so the +// wildcard doesn't silently admit executables/scripts. +export const BLOCKED_EXTENSIONS = [ + // Server-rendered / scripted content that could XSS a viewer + '.svg', '.html', '.htm', '.xml', '.xhtml', + // Scripts + '.js', '.jsx', '.ts', '.tsx', '.mjs', '.cjs', '.php', '.py', '.rb', '.pl', + // Executables + '.exe', '.bat', '.sh', '.cmd', '.msi', '.dll', '.com', '.vbs', '.ps1', '.app', +]; +// One directory level deeper than the legacy src/services/fileService.ts, so +// the extra '..' keeps the same absolute /uploads/files under both the +// src (vitest) and dist (runtime) layouts. +export const filesDir = path.join(__dirname, '../../../uploads/files'); diff --git a/server/src/nest/files/files.controller.ts b/server/src/nest/files/files.controller.ts index 669f5fa02..372348062 100644 --- a/server/src/nest/files/files.controller.ts +++ b/server/src/nest/files/files.controller.ts @@ -25,7 +25,9 @@ import type { User } from '../../types'; import { FilesService } from './files.service'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { MAX_FILE_SIZE, MAX_VIDEO_SIZE, BLOCKED_EXTENSIONS, filesDir, getAllowedExtensions, isVideoExtension } from '../../services/fileService'; +import { MAX_FILE_SIZE, MAX_VIDEO_SIZE, BLOCKED_EXTENSIONS, filesDir, isVideoExtension } from './files.constants'; +import { getAllowedExtensions } from './files.bridge'; +import { FileUploadDto, FileUpdateDto, FileLinkDto } from './files.dto'; import { isDemoEmail } from '../../services/demo'; const UPLOAD = { @@ -80,7 +82,7 @@ export class FilesController { // A file may only point at reservations/assignments/places from its own trip. // Reject cross-trip ids before they are stored — the reservation JOIN would // otherwise leak the foreign reservation's title back to the caller. - private assertLinkTargets(tripId: string, body: { reservation_id?: string | null; assignment_id?: string | null; place_id?: string | null }) { + private assertLinkTargets(tripId: string, body: { reservation_id?: string | number | null; assignment_id?: string | number | null; place_id?: string | number | null }) { if (this.files.findForeignLinkTarget(tripId, body)) { throw new HttpException({ error: 'Linked item does not belong to this trip' }, 400); } @@ -98,7 +100,7 @@ export class FilesController { @CurrentUser() user: User, @Param('tripId') tripId: string, @UploadedFile() file: Express.Multer.File | undefined, - @Body() body: { place_id?: string; description?: string; reservation_id?: string }, + @Body() body: FileUploadDto, @Headers('x-socket-id') socketId?: string, ) { // multer (diskStorage) has already written the upload by the time we get here, @@ -144,7 +146,7 @@ export class FilesController { } @Put(':id') - update(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body() body: { description?: string; place_id?: string | null; reservation_id?: string | null }, @Headers('x-socket-id') socketId?: string) { + update(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body() body: FileUpdateDto, @Headers('x-socket-id') socketId?: string) { const trip = this.requireTrip(tripId, user); if (!this.files.can('file_edit', trip, user)) { throw new HttpException({ error: 'No permission to edit files' }, 403); @@ -232,7 +234,7 @@ export class FilesController { @Post(':id/link') @HttpCode(200) // Express answers link with res.json (200). - link(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body() body: { reservation_id?: string | null; assignment_id?: string | null; place_id?: string | null }) { + link(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body() body: FileLinkDto) { const trip = this.requireTrip(tripId, user); if (!this.files.can('file_edit', trip, user)) { throw new HttpException({ error: 'No permission' }, 403); diff --git a/server/src/nest/files/files.dto.ts b/server/src/nest/files/files.dto.ts new file mode 100644 index 000000000..38b3e243a --- /dev/null +++ b/server/src/nest/files/files.dto.ts @@ -0,0 +1,18 @@ +import { createZodDto } from 'nestjs-zod'; +import { + fileUploadRequestSchema, + fileUpdateRequestSchema, + fileLinkRequestSchema, +} from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared file contracts. The + * global ZodValidationPipe (APP_PIPE in app.module.ts) validates any @Body() + * parameter typed with one of these classes by metatype — the Zod schemas in + * shared/ remain the single source of truth for the wire contract. Upload is + * multipart: FileInterceptor populates req.body with the text fields before + * the pipe runs, so the DTO sees plain strings. + */ +export class FileUploadDto extends createZodDto(fileUploadRequestSchema) {} +export class FileUpdateDto extends createZodDto(fileUpdateRequestSchema) {} +export class FileLinkDto extends createZodDto(fileLinkRequestSchema) {} diff --git a/server/src/nest/files/files.module.ts b/server/src/nest/files/files.module.ts index 160f404e4..a7b121dce 100644 --- a/server/src/nest/files/files.module.ts +++ b/server/src/nest/files/files.module.ts @@ -6,5 +6,6 @@ import { FilesService } from './files.service'; @Module({ controllers: [FilesController, FilesDownloadController], providers: [FilesService], + exports: [FilesService], }) export class FilesModule {} diff --git a/server/src/nest/files/files.service.ts b/server/src/nest/files/files.service.ts index 48ecfeb1e..b032e4dec 100644 --- a/server/src/nest/files/files.service.ts +++ b/server/src/nest/files/files.service.ts @@ -1,22 +1,63 @@ import { Injectable } from '@nestjs/common'; +import path from 'path'; +import fs from 'fs'; import type { Request } from 'express'; import { broadcast } from '../../websocket'; import { checkPermission } from '../../services/permissions'; +import { avatarUrl } from '../../services/avatarUrl'; +import { consumeEphemeralToken } from '../../services/ephemeralTokens'; +import { verifyJwtAndLoadUser } from '../../middleware/auth'; import type { User, TripFile } from '../../types'; -import * as svc from '../../services/fileService'; +import { DatabaseService, type TripAccess } from '../database/database.service'; +import { DEFAULT_ALLOWED_EXTENSIONS, filesDir } from './files.constants'; -type Trip = NonNullable>; +type Trip = TripAccess; type FilePermission = 'file_upload' | 'file_edit' | 'file_delete'; +const FILE_SELECT = ` + SELECT f.*, r.title as reservation_title, u.username as uploaded_by_name, u.avatar as uploaded_by_avatar + FROM trip_files f + LEFT JOIN reservations r ON f.reservation_id = r.id + LEFT JOIN users u ON f.uploaded_by = u.id +`; + +function formatFile(file: TripFile & { uploaded_by_avatar?: string | null }) { + const tripId = file.trip_id; + return { + ...file, + url: `/api/trips/${tripId}/files/${file.id}/download`, + uploaded_by_avatar: avatarUrl({ avatar: file.uploaded_by_avatar }), + }; +} + +export interface FileLink { + file_id: number; + reservation_id: number | null; + place_id: number | null; +} + /** - * Thin Nest wrapper around the existing file service. Trip access, the - * file_* permissions, the SQL, the path-resolution guard, the download-token - * auth and the WebSocket broadcasts reuse the legacy code unchanged. + * File domain service — owns the file SQL (moved 1:1 from the legacy + * services/fileService.ts: identical statements, the `||` falsy-coercion + * defaults, the post-write FILE_SELECT re-selects, the dynamic IN batches and + * the unlink-first delete semantics). Trip access goes through the injected + * DatabaseService's canAccessTrip (the legacy verifyTripAccess re-export was a + * plain wrapper over the same helper); the file_* permissions, path-resolution + * guard, download-token auth and WebSocket broadcasts are unchanged. The + * load-time constants live in files.constants.ts; the one out-of-DI consumer + * path (module-scope multer configs) reaches getAllowedExtensions through + * files.bridge.ts. + * + * Two deliberate post-migration fixes over the legacy behavior: createFileLink + * no longer swallows insert errors, and updateFile coerces an empty-string + * description to NULL exactly like createFile does. */ @Injectable() export class FilesService { - verifyTripAccess(tripId: string, userId: number) { - return svc.verifyTripAccess(tripId, userId); + constructor(private readonly db: DatabaseService) {} + + verifyTripAccess(tripId: string | number, userId: number) { + return this.db.canAccessTrip(tripId, userId); } can(action: FilePermission, trip: Trip, user: User): boolean { @@ -27,24 +68,247 @@ export class FilesService { broadcast(tripId, event, payload, socketId); } - // Download-token auth + safe path resolution (used by the unguarded download route). - authenticateDownload(req: Request) { return svc.authenticateDownload(req); } - resolveFilePath(filename: string) { return svc.resolveFilePath(filename); } - - listFiles(tripId: string, showTrash: boolean) { return svc.listFiles(tripId, showTrash); } - getFileById(id: string, tripId: string) { return svc.getFileById(id, tripId); } - getDeletedFile(id: string, tripId: string) { return svc.getDeletedFile(id, tripId); } - createFile(tripId: string, file: Parameters[1], userId: number, opts: Parameters[3]) { - return svc.createFile(tripId, file, userId, opts); + getAllowedExtensions(): string { + try { + const row = this.db.get<{ value: string }>("SELECT value FROM app_settings WHERE key = 'allowed_file_types'"); + return row?.value || DEFAULT_ALLOWED_EXTENSIONS; + } catch { return DEFAULT_ALLOWED_EXTENSIONS; } + } + + // --------------------------------------------------------------------------- + // Token-based download auth & safe path resolution (used by the unguarded + // download route) + // --------------------------------------------------------------------------- + + authenticateDownload(req: Request): { userId: number } | { error: string; status: number } { + const cookieToken = (req as { cookies?: Record }).cookies?.trek_session; + const authHeader = req.headers['authorization']; + const bearerToken = authHeader ? (authHeader.split(' ')[1] || undefined) : undefined; + const queryToken = req.query.token as string | undefined; + + // Cookie and Bearer both carry a full JWT — try them first (cookie wins). + const jwtToken = cookieToken || bearerToken; + if (jwtToken) { + // Use the shared helper so the password_version gate applies here too; + // previously this bypassed the check and stolen download tokens stayed + // valid across a password reset. + const user = verifyJwtAndLoadUser(jwtToken); + if (!user) return { error: 'Invalid or expired token', status: 401 }; + return { userId: user.id }; + } + + if (queryToken) { + const uid = consumeEphemeralToken(queryToken, 'download'); + if (!uid) return { error: 'Invalid or expired token', status: 401 }; + return { userId: uid }; + } + + return { error: 'Authentication required', status: 401 }; + } + + resolveFilePath(filename: string): { resolved: string; safe: boolean } { + const safeName = path.basename(filename); + const filePath = path.join(filesDir, safeName); + const resolved = path.resolve(filePath); + const safe = resolved.startsWith(path.resolve(filesDir)); + return { resolved, safe }; + } + + // --------------------------------------------------------------------------- + // Trip-scoped link validation + // --------------------------------------------------------------------------- + + /** + * A file, and any reservation / day-assignment / place it points at, must all + * live in the same trip. FILE_SELECT and getFileLinks join the reservation and + * return its title, so without this guard a member of trip A could aim a file + * (or a file_link) at trip B's reservation id and read the title back. Returns + * the first field that escapes `tripId`, or null when every supplied id belongs + * to the trip. Absent / null / zero ids are ignored (they clear the link). + */ + findForeignLinkTarget( + tripId: string | number, + opts: { reservation_id?: string | number | null; assignment_id?: string | number | null; place_id?: string | number | null } + ): 'reservation_id' | 'assignment_id' | 'place_id' | null { + if (opts.reservation_id && !this.db.get('SELECT 1 FROM reservations WHERE id = ? AND trip_id = ?', opts.reservation_id, tripId)) { + return 'reservation_id'; + } + if (opts.place_id && !this.db.get('SELECT 1 FROM places WHERE id = ? AND trip_id = ?', opts.place_id, tripId)) { + return 'place_id'; + } + if (opts.assignment_id && !this.db.get('SELECT 1 FROM day_assignments a JOIN days d ON a.day_id = d.id WHERE a.id = ? AND d.trip_id = ?', opts.assignment_id, tripId)) { + return 'assignment_id'; + } + return null; + } + + // --------------------------------------------------------------------------- + // CRUD + // --------------------------------------------------------------------------- + + getFileById(id: string | number, tripId: string | number): TripFile | undefined { + return this.db.get('SELECT * FROM trip_files WHERE id = ? AND trip_id = ?', id, tripId); + } + + getDeletedFile(id: string | number, tripId: string | number): TripFile | undefined { + return this.db.get('SELECT * FROM trip_files WHERE id = ? AND trip_id = ? AND deleted_at IS NOT NULL', id, tripId); + } + + listFiles(tripId: string | number, showTrash: boolean) { + const where = showTrash ? 'f.trip_id = ? AND f.deleted_at IS NOT NULL' : 'f.trip_id = ? AND f.deleted_at IS NULL'; + const files = this.db.all(`${FILE_SELECT} WHERE ${where} ORDER BY f.starred DESC, f.created_at DESC`, tripId); + + const fileIds = files.map(f => f.id); + const linksMap: Record = {}; + if (fileIds.length > 0) { + const placeholders = fileIds.map(() => '?').join(','); + const links = this.db.all(`SELECT file_id, reservation_id, place_id FROM file_links WHERE file_id IN (${placeholders})`, ...fileIds); + for (const link of links) { + if (!linksMap[link.file_id]) linksMap[link.file_id] = []; + linksMap[link.file_id].push(link); + } + } + + return files.map(f => { + const fileLinks = linksMap[f.id] || []; + return { + ...formatFile(f), + linked_reservation_ids: fileLinks.filter(l => l.reservation_id).map(l => l.reservation_id), + linked_place_ids: fileLinks.filter(l => l.place_id).map(l => l.place_id), + }; + }); + } + + createFile( + tripId: string | number, + file: { filename: string; originalname: string; size: number; mimetype: string }, + uploadedBy: number, + opts: { place_id?: string | number | null; reservation_id?: string | number | null; description?: string | null } + ) { + const result = this.db.run(` + INSERT INTO trip_files (trip_id, place_id, reservation_id, filename, original_name, file_size, mime_type, description, uploaded_by) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + `, + tripId, + opts.place_id || null, + opts.reservation_id || null, + file.filename, + file.originalname, + file.size, + file.mimetype, + opts.description || null, + uploadedBy + ); + + const created = this.db.get(`${FILE_SELECT} WHERE f.id = ?`, result.lastInsertRowid)!; + return formatFile(created); + } + + updateFile( + id: string | number, + current: TripFile, + updates: { description?: string; place_id?: string | number | null; reservation_id?: string | number | null } + ) { + this.db.run(` + UPDATE trip_files SET + description = ?, + place_id = ?, + reservation_id = ? + WHERE id = ? + `, + updates.description !== undefined ? (updates.description || null) : current.description, + updates.place_id !== undefined ? (updates.place_id || null) : current.place_id, + updates.reservation_id !== undefined ? (updates.reservation_id || null) : current.reservation_id, + id + ); + + const updated = this.db.get(`${FILE_SELECT} WHERE f.id = ?`, id)!; + return formatFile(updated); + } + + toggleStarred(id: string | number, currentStarred: number | undefined) { + const newStarred = currentStarred ? 0 : 1; + this.db.run('UPDATE trip_files SET starred = ? WHERE id = ?', newStarred, id); + + const updated = this.db.get(`${FILE_SELECT} WHERE f.id = ?`, id)!; + return formatFile(updated); + } + + softDeleteFile(id: string | number) { + this.db.run('UPDATE trip_files SET deleted_at = CURRENT_TIMESTAMP WHERE id = ?', id); + } + + restoreFile(id: string | number) { + this.db.run('UPDATE trip_files SET deleted_at = NULL WHERE id = ?', id); + const restored = this.db.get(`${FILE_SELECT} WHERE f.id = ?`, id)!; + return formatFile(restored); + } + + async permanentDeleteFile(file: TripFile): Promise { + const { resolved } = this.resolveFilePath(file.filename); + // `force: true` swallows ENOENT, replacing the prior existsSync+unlink + // double-call that blocked the event loop twice per deletion. Only + // drop the DB row when the on-disk unlink either succeeded or the + // file was already gone — otherwise a permission / ENOSPC failure + // would orphan the bytes on disk with no DB pointer left to clean it. + try { + await fs.promises.rm(resolved, { force: true }); + } catch (e) { + console.error(`[files] unlink failed for ${file.filename}, keeping DB row:`, e); + throw e; + } + this.db.run('DELETE FROM trip_files WHERE id = ?', file.id); + } + + async emptyTrash(tripId: string | number): Promise { + const trashed = this.db.all('SELECT * FROM trip_files WHERE trip_id = ? AND deleted_at IS NOT NULL', tripId); + // Collect successful IDs separately so we only DELETE rows whose disk + // content was actually removed — failing unlinks keep their DB row + // and a retry via the single-file delete path can try again. + const successfullyUnlinked: number[] = []; + await Promise.all(trashed.map(async (file) => { + const { resolved } = this.resolveFilePath(file.filename); + try { + await fs.promises.rm(resolved, { force: true }); + successfullyUnlinked.push(Number(file.id)); + } catch (e) { + console.error(`[files] unlink failed for ${file.filename}, keeping DB row:`, e); + } + })); + if (successfullyUnlinked.length > 0) { + const placeholders = successfullyUnlinked.map(() => '?').join(','); + this.db.run(`DELETE FROM trip_files WHERE id IN (${placeholders})`, ...successfullyUnlinked); + } + return successfullyUnlinked.length; + } + + // --------------------------------------------------------------------------- + // File links (many-to-many) + // --------------------------------------------------------------------------- + + // Dedupe rides INSERT OR IGNORE + the UNIQUE(file_id, ) constraints; + // a genuine insert failure propagates to the global exception filter instead + // of returning a success-shaped links list (the legacy catch swallowed it). + createFileLink( + fileId: string | number, + opts: { reservation_id?: string | number | null; assignment_id?: string | number | null; place_id?: string | number | null } + ) { + this.db.run('INSERT OR IGNORE INTO file_links (file_id, reservation_id, assignment_id, place_id) VALUES (?, ?, ?, ?)', + fileId, opts.reservation_id || null, opts.assignment_id || null, opts.place_id || null + ); + return this.db.all('SELECT * FROM file_links WHERE file_id = ?', fileId); + } + + deleteFileLink(linkId: string | number, fileId: string | number) { + this.db.run('DELETE FROM file_links WHERE id = ? AND file_id = ?', linkId, fileId); + } + + getFileLinks(fileId: string | number) { + return this.db.all(` + SELECT fl.*, r.title as reservation_title + FROM file_links fl + LEFT JOIN reservations r ON fl.reservation_id = r.id + WHERE fl.file_id = ? + `, fileId); } - updateFile(id: string, current: TripFile, updates: Parameters[2]) { return svc.updateFile(id, current, updates); } - toggleStarred(id: string, currentStarred: number | undefined) { return svc.toggleStarred(id, currentStarred); } - softDeleteFile(id: string) { return svc.softDeleteFile(id); } - restoreFile(id: string) { return svc.restoreFile(id); } - permanentDeleteFile(file: TripFile) { return svc.permanentDeleteFile(file); } - emptyTrash(tripId: string) { return svc.emptyTrash(tripId); } - findForeignLinkTarget(tripId: string, opts: Parameters[1]) { return svc.findForeignLinkTarget(tripId, opts); } - createFileLink(id: string, opts: Parameters[1]) { return svc.createFileLink(id, opts); } - deleteFileLink(linkId: string, id: string) { return svc.deleteFileLink(linkId, id); } - getFileLinks(id: string) { return svc.getFileLinks(id); } } diff --git a/server/src/nest/journey/journey.controller.ts b/server/src/nest/journey/journey.controller.ts index 48cc11ad9..c0c13ab72 100644 --- a/server/src/nest/journey/journey.controller.ts +++ b/server/src/nest/journey/journey.controller.ts @@ -25,7 +25,8 @@ import { JourneyService } from './journey.service'; import { JourneyAddonGuard } from './journey-addon.guard'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { getAllowedExtensions, isVideoMime, isVideoExtension, MAX_VIDEO_SIZE } from '../../services/fileService'; +import { isVideoMime, isVideoExtension, MAX_VIDEO_SIZE } from '../files/files.constants'; +import { getAllowedExtensions } from '../files/files.bridge'; const uploadsBase = path.join(__dirname, '../../../uploads/journey'); const IMAGE_UPLOAD = { diff --git a/server/src/nest/plugins/host/plugin-host-deps.factory.ts b/server/src/nest/plugins/host/plugin-host-deps.factory.ts index ec2f40a22..defcc3930 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -4,7 +4,7 @@ import { broadcast, broadcastToUser } from '../../../websocket'; import { listBudgetItems } from '../../../services/budgetService'; import { isUpdateConflict } from '../../../services/conflictResult'; import { getWeather } from '../../../services/weatherService'; -import { listFiles, createFile, createFileLink, getFileById, updateFile, softDeleteFile, findForeignLinkTarget, resolveFilePath, BLOCKED_EXTENSIONS, filesDir } from '../../../services/fileService'; +import { BLOCKED_EXTENSIONS, filesDir } from '../../files/files.constants'; import { createNote as createCollabNoteSvc, createPoll as createCollabPollSvc, votePoll as voteCollabPollSvc, createMessage as createCollabMessageSvc, listNotes as listCollabNotesSvc, listPolls as listCollabPollsSvc, listMessages as listCollabMessagesSvc } from '../../../services/collabService'; import { getRates as getExchangeRates } from '../../../services/exchangeRateService'; import { joinTripAsMember } from '../../../services/tripMembership'; @@ -36,6 +36,7 @@ import { DayNotesService } from '../../days/day-notes.service'; import { AssignmentsService } from '../../assignments/assignments.service'; import { LlmConfigResolver } from '../../llm-parse/llm-config.resolver'; import { DatabaseService } from '../../database/database.service'; +import { FilesService } from '../../files/files.service'; import { notifyBookingChange } from '../../../services/reservationService'; import { PluginRpcHost, ForbiddenResource, BadParams } from './rpc-host'; import { appendAudit } from './plugin-audit'; @@ -136,8 +137,8 @@ export interface PluginCallRouter { * `plugin:{id}:{event}` so a plugin can't forge a core event. * * DI-native domain services (budget, reservations, tags, categories, todo, - * packing, day-notes, assignments, oauth, the LLM config resolver) and the - * DatabaseService (all inline SQL + the trip-access helper) are + * packing, day-notes, assignments, oauth, files, the LLM config resolver) and + * the DatabaseService (all inline SQL + the trip-access helper) are * constructor-injected; legacy `services/*` domains stay plain function * imports until their own migration lands (DI-MIGRATION.md), at which point * each swaps one import for one injected service. @@ -156,6 +157,7 @@ export class PluginHostDepsFactory { private readonly assignments: AssignmentsService, private readonly llmConfig: LlmConfigResolver, private readonly db: DatabaseService, + private readonly files: FilesService, ) {} /** @@ -217,16 +219,16 @@ export class PluginHostDepsFactory { // --- Read scopes (packing/files). Membership is checked by the host (tripRead); // these just delegate to the same services the REST paths use. --- listPackingItems: (tripId, userId) => this.packing.listItems(tripId, userId), - listTripFiles: (tripId) => listFiles(tripId, false), + listTripFiles: (tripId) => this.files.listFiles(tripId, false), // A file's bytes as base64, size-capped BEFORE the read so a 500MB video can't // be pulled (~667MB base64) through the IPC pipe. 10MB matches the plugin upload // cap; trashed files (deleted_at set) are refused like the download path. getTripFileContent: async (tripId, fileId) => { const CONTENT_MAX = 10 * 1024 * 1024; - const file = getFileById(fileId, tripId) as { filename: string; original_name: string; mime_type: string | null; file_size: number | null; deleted_at: string | null } | undefined; + const file = this.files.getFileById(fileId, tripId) as { filename: string; original_name: string; mime_type: string | null; file_size: number | null; deleted_at: string | null } | undefined; if (!file || file.deleted_at) throw new ForbiddenResource(`no file ${fileId} on trip ${tripId}`); if ((file.file_size ?? 0) > CONTENT_MAX) throw new BadParams(`file too large to read (>${CONTENT_MAX} bytes); use the download UI`); - const { resolved, safe } = resolveFilePath(file.filename); + const { resolved, safe } = this.files.resolveFilePath(file.filename); if (!safe) throw new ForbiddenResource('file path is not accessible'); // Read off the event loop — a 10MB read + base64 on the host thread would // otherwise stall every other plugin RPC and request for its duration. @@ -256,12 +258,12 @@ export class PluginHostDepsFactory { const buf = Buffer.from(i.content_base64, 'base64'); if (buf.length === 0) throw new BadParams('file content is empty'); if (buf.length > 10 * 1024 * 1024) throw new BadParams('file exceeds the 10MB plugin upload cap'); - const foreign = findForeignLinkTarget(tripId, { reservation_id: i.reservation_id ?? null, place_id: i.place_id ?? null }); + const foreign = this.files.findForeignLinkTarget(tripId, { reservation_id: i.reservation_id ?? null, place_id: i.place_id ?? null }); if (foreign) throw new ForbiddenResource(`${foreign} does not belong to trip ${tripId}`); const filename = `${randomUUID()}${ext}`; fsMod.mkdirSync(filesDir, { recursive: true }); fsMod.writeFileSync(pathMod.join(filesDir, filename), buf); - const file = createFile( + const file = this.files.createFile( tripId, { filename, originalname: original, size: buf.length, mimetype: i.mimetype || 'application/octet-stream' }, actingUserId, @@ -271,25 +273,25 @@ export class PluginHostDepsFactory { return file; }, createTripFileLink: (tripId, fileId, opts) => { - if (!getFileById(fileId, tripId)) throw new ForbiddenResource(`no file ${fileId} on trip ${tripId}`); + if (!this.files.getFileById(fileId, tripId)) throw new ForbiddenResource(`no file ${fileId} on trip ${tripId}`); const o = opts as { reservation_id?: number; assignment_id?: number; place_id?: number }; - const foreign = findForeignLinkTarget(tripId, o); + const foreign = this.files.findForeignLinkTarget(tripId, o); if (foreign) throw new ForbiddenResource(`${foreign} does not belong to trip ${tripId}`); - return createFileLink(fileId, { reservation_id: o.reservation_id != null ? String(o.reservation_id) : null, assignment_id: o.assignment_id != null ? String(o.assignment_id) : null, place_id: o.place_id != null ? String(o.place_id) : null }); + return this.files.createFileLink(fileId, { reservation_id: o.reservation_id != null ? String(o.reservation_id) : null, assignment_id: o.assignment_id != null ? String(o.assignment_id) : null, place_id: o.place_id != null ? String(o.place_id) : null }); }, updateTripFile: (tripId, fileId, input) => { - const current = getFileById(fileId, tripId); + const current = this.files.getFileById(fileId, tripId); if (!current) throw new ForbiddenResource(`no file ${fileId} on trip ${tripId}`); const i = input as { description?: string; place_id?: number | null; reservation_id?: number | null }; - const foreign = findForeignLinkTarget(tripId, { reservation_id: i.reservation_id ?? null, place_id: i.place_id ?? null }); + const foreign = this.files.findForeignLinkTarget(tripId, { reservation_id: i.reservation_id ?? null, place_id: i.place_id ?? null }); if (foreign) throw new ForbiddenResource(`${foreign} does not belong to trip ${tripId}`); - const file = updateFile(fileId, current, { description: i.description, place_id: i.place_id != null ? String(i.place_id) : i.place_id === null ? null : undefined, reservation_id: i.reservation_id != null ? String(i.reservation_id) : i.reservation_id === null ? null : undefined }); + const file = this.files.updateFile(fileId, current, { description: i.description, place_id: i.place_id != null ? String(i.place_id) : i.place_id === null ? null : undefined, reservation_id: i.reservation_id != null ? String(i.reservation_id) : i.reservation_id === null ? null : undefined }); broadcast(tripId, 'file:updated', { file }, undefined); return file; }, softDeleteTripFile: (tripId, fileId) => { - if (!getFileById(fileId, tripId)) throw new ForbiddenResource(`no file ${fileId} on trip ${tripId}`); - softDeleteFile(fileId); + if (!this.files.getFileById(fileId, tripId)) throw new ForbiddenResource(`no file ${fileId} on trip ${tripId}`); + this.files.softDeleteFile(fileId); broadcast(tripId, 'file:deleted', { fileId }, undefined); return { deleted: true }; }, diff --git a/server/src/nest/plugins/plugins.module.ts b/server/src/nest/plugins/plugins.module.ts index 555268749..2d0e6bfb4 100644 --- a/server/src/nest/plugins/plugins.module.ts +++ b/server/src/nest/plugins/plugins.module.ts @@ -33,6 +33,7 @@ import { PackingModule } from '../packing/packing.module'; import { DaysModule } from '../days/days.module'; import { AssignmentsModule } from '../assignments/assignments.module'; import { LlmParseModule } from '../llm-parse/llm-parse.module'; +import { FilesModule } from '../files/files.module'; /** * Plugin system (#plugins). M0 read side + M2 isolated runtime + M3 frontend: @@ -44,7 +45,7 @@ import { LlmParseModule } from '../llm-parse/llm-parse.module'; @Module({ // The DI-native domain services the plugin host wiring injects // (PluginHostDepsFactory); DatabaseModule is @Global, so not listed. - imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule], + imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule, FilesModule], controllers: [PluginsController, PluginsFeedController, PluginsProxyController, PluginFrameController, PlaceDetailsController, TripWarningsController, ViewContributionsController, TripCardContributionsController, PluginPhotosController, PluginCalendarController, MapMarkersController, MapLayersController, PluginRoutesController, DayScheduleController, PdfSectionsController, AtlasLayersController, JournalEntryRowsController, PluginUserSettingsController, PluginOAuthController, PluginActivityController], providers: [PluginsService, PluginRuntimeService, PluginRegistryService, PluginOAuthService, PluginHostDepsFactory], // Exported so the admin addon-toggle handler can cascade-disable plugins whose diff --git a/server/src/nest/trips/trips.module.ts b/server/src/nest/trips/trips.module.ts index 89466296e..ddc97a31f 100644 --- a/server/src/nest/trips/trips.module.ts +++ b/server/src/nest/trips/trips.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { FilesModule } from '../files/files.module'; import { PackingModule } from '../packing/packing.module'; import { TodoModule } from '../todo/todo.module'; import { TripsController } from './trips.controller'; @@ -7,7 +8,7 @@ import { TripsService } from './trips.service'; /** Trips aggregate root (C1 — Phase 3). Uses exact strangler prefixes so it does * not capture the nested sub-domain mounts (collab, files, ...). */ @Module({ - imports: [TodoModule, PackingModule], + imports: [TodoModule, PackingModule, FilesModule], controllers: [TripsController], providers: [TripsService], }) diff --git a/server/src/nest/trips/trips.service.ts b/server/src/nest/trips/trips.service.ts index 7dae9783e..fdf048537 100644 --- a/server/src/nest/trips/trips.service.ts +++ b/server/src/nest/trips/trips.service.ts @@ -10,7 +10,7 @@ import { PackingService } from '../packing/packing.service'; import { TodoService } from '../todo/todo.service'; import { listBudgetItems, rebaseTripCurrency } from '../../services/budgetService'; import { listReservations } from '../../services/reservationService'; -import { listFiles } from '../../services/fileService'; +import { FilesService } from '../files/files.service'; import { searchUnsplashPhotos, getUnsplashKey } from '../../services/unsplashService'; /** @@ -26,6 +26,7 @@ export class TripsService { private readonly dbs: DatabaseService, private readonly todo: TodoService, private readonly packing: PackingService, + private readonly files: FilesService, ) {} private get db() { @@ -143,7 +144,7 @@ export class TripsService { todoItems: this.todo.listItems(tripId), budgetItems: listBudgetItems(tripId), reservations: listReservations(tripId), - files: listFiles(tripId, false), + files: this.files.listFiles(tripId, false), accommodations: listAccommodations(tripId), members: [owner, ...(members || [])].filter(Boolean), }; diff --git a/server/src/services/fileService.ts b/server/src/services/fileService.ts deleted file mode 100644 index 13d6251d2..000000000 --- a/server/src/services/fileService.ts +++ /dev/null @@ -1,319 +0,0 @@ -import path from 'path'; -import { avatarUrl } from './avatarUrl'; -import fs from 'fs'; -import type { Request } from 'express'; -import { db } from '../db/database'; -import { consumeEphemeralToken } from './ephemeralTokens'; -import { verifyJwtAndLoadUser } from '../middleware/auth'; -import { TripFile } from '../types'; - -// --------------------------------------------------------------------------- -// Constants -// --------------------------------------------------------------------------- - -export const MAX_FILE_SIZE = 50 * 1024 * 1024; // 50 MB -export const DEFAULT_ALLOWED_EXTENSIONS = 'jpg,jpeg,png,gif,webp,heic,pdf,doc,docx,xls,xlsx,txt,csv,pkpass,pkpasses,md,markdown'; - -// Video support (#823). Gallery/media uploads accept these in addition to images, -// independent of the admin doc-types allowlist. Videos are stored as-is and -// streamed with HTTP Range; the cap is higher than images because phone clips are -// large. -export const VIDEO_EXTENSIONS = ['mp4', 'm4v', 'webm', 'mov']; -export const MAX_VIDEO_SIZE = 500 * 1024 * 1024; // 500 MB - -export function isVideoMime(mime: string | undefined | null): boolean { - return !!mime && mime.startsWith('video/'); -} - -export function isVideoExtension(ext: string): boolean { - return VIDEO_EXTENSIONS.includes(ext.toLowerCase().replace(/^\./, '')); -} -// Single authoritative blocklist for every file-upload surface (main -// file manager + collab attachments). When the admin setting -// `allowed_file_types` is `*`, this list is still enforced so the -// wildcard doesn't silently admit executables/scripts. -export const BLOCKED_EXTENSIONS = [ - // Server-rendered / scripted content that could XSS a viewer - '.svg', '.html', '.htm', '.xml', '.xhtml', - // Scripts - '.js', '.jsx', '.ts', '.tsx', '.mjs', '.cjs', '.php', '.py', '.rb', '.pl', - // Executables - '.exe', '.bat', '.sh', '.cmd', '.msi', '.dll', '.com', '.vbs', '.ps1', '.app', -]; -export const filesDir = path.join(__dirname, '../../uploads/files'); - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -export { verifyTripAccess } from './tripAccess'; - -export function getAllowedExtensions(): string { - try { - const row = db.prepare("SELECT value FROM app_settings WHERE key = 'allowed_file_types'").get() as { value: string } | undefined; - return row?.value || DEFAULT_ALLOWED_EXTENSIONS; - } catch { return DEFAULT_ALLOWED_EXTENSIONS; } -} - -const FILE_SELECT = ` - SELECT f.*, r.title as reservation_title, u.username as uploaded_by_name, u.avatar as uploaded_by_avatar - FROM trip_files f - LEFT JOIN reservations r ON f.reservation_id = r.id - LEFT JOIN users u ON f.uploaded_by = u.id -`; - -export function formatFile(file: TripFile & { trip_id?: number; uploaded_by_avatar?: string | null }) { - const tripId = file.trip_id; - return { - ...file, - url: `/api/trips/${tripId}/files/${file.id}/download`, - uploaded_by_avatar: avatarUrl({ avatar: file.uploaded_by_avatar }), - }; -} - -// --------------------------------------------------------------------------- -// Trip-scoped link validation -// --------------------------------------------------------------------------- - -/** - * A file, and any reservation / day-assignment / place it points at, must all - * live in the same trip. FILE_SELECT and getFileLinks join the reservation and - * return its title, so without this guard a member of trip A could aim a file - * (or a file_link) at trip B's reservation id and read the title back. Returns - * the first field that escapes `tripId`, or null when every supplied id belongs - * to the trip. Absent / null / zero ids are ignored (they clear the link). - */ -export function findForeignLinkTarget( - tripId: string | number, - opts: { reservation_id?: string | number | null; assignment_id?: string | number | null; place_id?: string | number | null } -): 'reservation_id' | 'assignment_id' | 'place_id' | null { - if (opts.reservation_id && !db.prepare('SELECT 1 FROM reservations WHERE id = ? AND trip_id = ?').get(opts.reservation_id, tripId)) { - return 'reservation_id'; - } - if (opts.place_id && !db.prepare('SELECT 1 FROM places WHERE id = ? AND trip_id = ?').get(opts.place_id, tripId)) { - return 'place_id'; - } - if (opts.assignment_id && !db.prepare('SELECT 1 FROM day_assignments a JOIN days d ON a.day_id = d.id WHERE a.id = ? AND d.trip_id = ?').get(opts.assignment_id, tripId)) { - return 'assignment_id'; - } - return null; -} - -// --------------------------------------------------------------------------- -// File path resolution & validation -// --------------------------------------------------------------------------- - -export function resolveFilePath(filename: string): { resolved: string; safe: boolean } { - const safeName = path.basename(filename); - const filePath = path.join(filesDir, safeName); - const resolved = path.resolve(filePath); - const safe = resolved.startsWith(path.resolve(filesDir)); - return { resolved, safe }; -} - -// --------------------------------------------------------------------------- -// Token-based download auth -// --------------------------------------------------------------------------- - -export function authenticateDownload(req: Request): { userId: number } | { error: string; status: number } { - const cookieToken = (req as any).cookies?.trek_session as string | undefined; - const authHeader = req.headers['authorization']; - const bearerToken = authHeader ? (authHeader.split(' ')[1] || undefined) : undefined; - const queryToken = req.query.token as string | undefined; - - // Cookie and Bearer both carry a full JWT — try them first (cookie wins). - const jwtToken = cookieToken || bearerToken; - if (jwtToken) { - // Use the shared helper so the password_version gate applies here too; - // previously this bypassed the check and stolen download tokens stayed - // valid across a password reset. - const user = verifyJwtAndLoadUser(jwtToken); - if (!user) return { error: 'Invalid or expired token', status: 401 }; - return { userId: user.id }; - } - - if (queryToken) { - const uid = consumeEphemeralToken(queryToken, 'download'); - if (!uid) return { error: 'Invalid or expired token', status: 401 }; - return { userId: uid }; - } - - return { error: 'Authentication required', status: 401 }; -} - -// --------------------------------------------------------------------------- -// CRUD -// --------------------------------------------------------------------------- - -export interface FileLink { - file_id: number; - reservation_id: number | null; - place_id: number | null; -} - -export function getFileById(id: string | number, tripId: string | number): TripFile | undefined { - return db.prepare('SELECT * FROM trip_files WHERE id = ? AND trip_id = ?').get(id, tripId) as TripFile | undefined; -} - -export function getDeletedFile(id: string | number, tripId: string | number): TripFile | undefined { - return db.prepare('SELECT * FROM trip_files WHERE id = ? AND trip_id = ? AND deleted_at IS NOT NULL').get(id, tripId) as TripFile | undefined; -} - -export function listFiles(tripId: string | number, showTrash: boolean) { - const where = showTrash ? 'f.trip_id = ? AND f.deleted_at IS NOT NULL' : 'f.trip_id = ? AND f.deleted_at IS NULL'; - const files = db.prepare(`${FILE_SELECT} WHERE ${where} ORDER BY f.starred DESC, f.created_at DESC`).all(tripId) as TripFile[]; - - const fileIds = files.map(f => f.id); - const linksMap: Record = {}; - if (fileIds.length > 0) { - const placeholders = fileIds.map(() => '?').join(','); - const links = db.prepare(`SELECT file_id, reservation_id, place_id FROM file_links WHERE file_id IN (${placeholders})`).all(...fileIds) as FileLink[]; - for (const link of links) { - if (!linksMap[link.file_id]) linksMap[link.file_id] = []; - linksMap[link.file_id].push(link); - } - } - - return files.map(f => { - const fileLinks = linksMap[f.id] || []; - return { - ...formatFile(f), - linked_reservation_ids: fileLinks.filter(l => l.reservation_id).map(l => l.reservation_id), - linked_place_ids: fileLinks.filter(l => l.place_id).map(l => l.place_id), - }; - }); -} - -export function createFile( - tripId: string | number, - file: { filename: string; originalname: string; size: number; mimetype: string }, - uploadedBy: number, - opts: { place_id?: string | null; reservation_id?: string | null; description?: string | null } -) { - const result = db.prepare(` - INSERT INTO trip_files (trip_id, place_id, reservation_id, filename, original_name, file_size, mime_type, description, uploaded_by) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) - `).run( - tripId, - opts.place_id || null, - opts.reservation_id || null, - file.filename, - file.originalname, - file.size, - file.mimetype, - opts.description || null, - uploadedBy - ); - - const created = db.prepare(`${FILE_SELECT} WHERE f.id = ?`).get(result.lastInsertRowid) as TripFile; - return formatFile(created); -} - -export function updateFile( - id: string | number, - current: TripFile, - updates: { description?: string; place_id?: string | null; reservation_id?: string | null } -) { - db.prepare(` - UPDATE trip_files SET - description = ?, - place_id = ?, - reservation_id = ? - WHERE id = ? - `).run( - updates.description !== undefined ? updates.description : current.description, - updates.place_id !== undefined ? (updates.place_id || null) : current.place_id, - updates.reservation_id !== undefined ? (updates.reservation_id || null) : current.reservation_id, - id - ); - - const updated = db.prepare(`${FILE_SELECT} WHERE f.id = ?`).get(id) as TripFile; - return formatFile(updated); -} - -export function toggleStarred(id: string | number, currentStarred: number | undefined) { - const newStarred = currentStarred ? 0 : 1; - db.prepare('UPDATE trip_files SET starred = ? WHERE id = ?').run(newStarred, id); - - const updated = db.prepare(`${FILE_SELECT} WHERE f.id = ?`).get(id) as TripFile; - return formatFile(updated); -} - -export function softDeleteFile(id: string | number) { - db.prepare('UPDATE trip_files SET deleted_at = CURRENT_TIMESTAMP WHERE id = ?').run(id); -} - -export function restoreFile(id: string | number) { - db.prepare('UPDATE trip_files SET deleted_at = NULL WHERE id = ?').run(id); - const restored = db.prepare(`${FILE_SELECT} WHERE f.id = ?`).get(id) as TripFile; - return formatFile(restored); -} - -export async function permanentDeleteFile(file: TripFile): Promise { - const { resolved } = resolveFilePath(file.filename); - // `force: true` swallows ENOENT, replacing the prior existsSync+unlink - // double-call that blocked the event loop twice per deletion. Only - // drop the DB row when the on-disk unlink either succeeded or the - // file was already gone — otherwise a permission / ENOSPC failure - // would orphan the bytes on disk with no DB pointer left to clean it. - try { - await fs.promises.rm(resolved, { force: true }); - } catch (e) { - console.error(`[files] unlink failed for ${file.filename}, keeping DB row:`, e); - throw e; - } - db.prepare('DELETE FROM trip_files WHERE id = ?').run(file.id); -} - -export async function emptyTrash(tripId: string | number): Promise { - const trashed = db.prepare('SELECT * FROM trip_files WHERE trip_id = ? AND deleted_at IS NOT NULL').all(tripId) as TripFile[]; - // Collect successful IDs separately so we only DELETE rows whose disk - // content was actually removed — failing unlinks keep their DB row - // and a retry via the single-file delete path can try again. - const successfullyUnlinked: number[] = []; - await Promise.all(trashed.map(async (file) => { - const { resolved } = resolveFilePath(file.filename); - try { - await fs.promises.rm(resolved, { force: true }); - successfullyUnlinked.push(Number(file.id)); - } catch (e) { - console.error(`[files] unlink failed for ${file.filename}, keeping DB row:`, e); - } - })); - if (successfullyUnlinked.length > 0) { - const placeholders = successfullyUnlinked.map(() => '?').join(','); - db.prepare(`DELETE FROM trip_files WHERE id IN (${placeholders})`).run(...successfullyUnlinked); - } - return successfullyUnlinked.length; -} - -// --------------------------------------------------------------------------- -// File links (many-to-many) -// --------------------------------------------------------------------------- - -export function createFileLink( - fileId: string | number, - opts: { reservation_id?: string | null; assignment_id?: string | null; place_id?: string | null } -) { - try { - db.prepare('INSERT OR IGNORE INTO file_links (file_id, reservation_id, assignment_id, place_id) VALUES (?, ?, ?, ?)').run( - fileId, opts.reservation_id || null, opts.assignment_id || null, opts.place_id || null - ); - } catch (err) { - console.error('[Files] Error creating file link:', err instanceof Error ? err.message : err); - } - return db.prepare('SELECT * FROM file_links WHERE file_id = ?').all(fileId); -} - -export function deleteFileLink(linkId: string | number, fileId: string | number) { - db.prepare('DELETE FROM file_links WHERE id = ? AND file_id = ?').run(linkId, fileId); -} - -export function getFileLinks(fileId: string | number) { - return db.prepare(` - SELECT fl.*, r.title as reservation_title - FROM file_links fl - LEFT JOIN reservations r ON fl.reservation_id = r.id - WHERE fl.file_id = ? - `).all(fileId); -} diff --git a/server/tests/e2e/files.e2e.test.ts b/server/tests/e2e/files.e2e.test.ts index d511cbd6d..614193b13 100644 --- a/server/tests/e2e/files.e2e.test.ts +++ b/server/tests/e2e/files.e2e.test.ts @@ -1,9 +1,11 @@ /** * Files + photos e2e — exercises the migrated /api/trips/:tripId/files and * /api/photos endpoints through the real JwtAuthGuard against a temp SQLite db. - * The file/photo services, permission check and broadcast are mocked; this - * focuses on auth (incl. the unguarded download's own token auth), trip-access - * 404, permission 403, the photo id/access guards and status codes. + * FilesService is DI-native (no service mock): the file rows live in the temp + * db and the SQL runs for real; only canAccessTrip, the permission check, the + * photo services and the broadcast are mocked. Focuses on auth (incl. the + * unguarded download's own token auth), trip-access 404, permission 403, the + * photo id/access guards and status codes. */ import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; import request from 'supertest'; @@ -18,29 +20,36 @@ const { db } = vi.hoisted(() => { const tmp = new Database(':memory:'); tmp.exec('PRAGMA journal_mode = WAL'); tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL, - email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0);`); + email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0, + avatar TEXT);`); + // FilesService runs its real SQL against these (FILE_SELECT joins reservations + // and users; the link batch reads file_links; findForeignLinkTarget probes + // reservations/places/day_assignments). + tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT);'); + tmp.exec(`CREATE TABLE trip_files (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, + place_id INTEGER, reservation_id INTEGER, filename TEXT NOT NULL, original_name TEXT NOT NULL, + file_size INTEGER, mime_type TEXT, description TEXT, uploaded_by INTEGER, starred INTEGER DEFAULT 0, + deleted_at DATETIME, created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); + tmp.exec(`CREATE TABLE file_links (id INTEGER PRIMARY KEY AUTOINCREMENT, file_id INTEGER NOT NULL, + reservation_id INTEGER, assignment_id INTEGER, place_id INTEGER, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); + tmp.exec('CREATE TABLE reservations (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER, title TEXT);'); + tmp.exec('CREATE TABLE places (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER);'); + tmp.exec('CREATE TABLE days (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER);'); + tmp.exec('CREATE TABLE day_assignments (id INTEGER PRIMARY KEY AUTOINCREMENT, day_id INTEGER);'); return { db: tmp }; }); -vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: () => {} })); +const { canAccessTrip } = vi.hoisted(() => ({ canAccessTrip: vi.fn() })); +vi.mock('../../src/db/database', () => ({ + db, canAccessTrip, isOwner: vi.fn(() => true), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, +})); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); vi.mock('../../src/services/demo', () => ({ isDemoEmail: vi.fn(() => false) })); const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); vi.mock('../../src/services/permissions', () => ({ checkPermission })); -const { fileSvc } = vi.hoisted(() => ({ - fileSvc: { - MAX_FILE_SIZE: 50 * 1024 * 1024, MAX_VIDEO_SIZE: 500 * 1024 * 1024, BLOCKED_EXTENSIONS: ['.exe', '.svg'], filesDir: '/tmp/files', getAllowedExtensions: () => '*', - isVideoExtension: (ext: string) => ['mp4', 'm4v', 'webm', 'mov'].includes(String(ext).toLowerCase().replace(/^\./, '')), isVideoMime: (m?: string) => !!m && m.startsWith('video/'), - verifyTripAccess: vi.fn(), resolveFilePath: vi.fn(), authenticateDownload: vi.fn(), - listFiles: vi.fn(), getFileById: vi.fn(), getDeletedFile: vi.fn(), createFile: vi.fn(), updateFile: vi.fn(), - toggleStarred: vi.fn(), softDeleteFile: vi.fn(), restoreFile: vi.fn(), permanentDeleteFile: vi.fn(), - emptyTrash: vi.fn(), createFileLink: vi.fn(), deleteFileLink: vi.fn(), getFileLinks: vi.fn(), formatFile: vi.fn(), - }, -})); -vi.mock('../../src/services/fileService', () => fileSvc); - const { photoSvc, helperSvc } = vi.hoisted(() => ({ photoSvc: { streamPhoto: vi.fn(), getPhotoInfo: vi.fn(), resolveTrekPhoto: vi.fn() }, helperSvc: { canAccessTrekPhoto: vi.fn() }, @@ -48,6 +57,7 @@ const { photoSvc, helperSvc } = vi.hoisted(() => ({ vi.mock('../../src/services/memories/photoResolverService', () => photoSvc); vi.mock('../../src/services/memories/helpersService', () => helperSvc); +import { DatabaseModule } from '../../src/nest/database/database.module'; import { FilesModule } from '../../src/nest/files/files.module'; import { PhotosModule } from '../../src/nest/photos/photos.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; @@ -57,7 +67,7 @@ describe('Files + photos e2e (real auth guard + temp SQLite)', () => { let app: Awaited>; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [FilesModule, PhotosModule] }).compile(); + const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, FilesModule, PhotosModule] }).compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); @@ -67,15 +77,15 @@ describe('Files + photos e2e (real auth guard + temp SQLite)', () => { beforeAll(async () => { seedUser(db as never, { id: 1 }); + db.prepare('INSERT INTO trips (id, user_id, title) VALUES (5, 1, ?)').run('Trip'); + db.prepare("INSERT INTO trip_files (id, trip_id, filename, original_name, uploaded_by) VALUES (1, 5, 'stored-a.pdf', 'a.pdf', 1)").run(); + db.prepare("INSERT INTO trip_files (id, trip_id, filename, original_name, uploaded_by, starred) VALUES (9, 5, 'stored-b.pdf', 'b.pdf', 1, 0)").run(); app = await build(); server = app.getHttpServer(); - fileSvc.listFiles.mockReturnValue([{ id: 1, original_name: 'a.pdf' }]); - fileSvc.getFileById.mockReturnValue({ id: 9, starred: 0 }); - fileSvc.toggleStarred.mockReturnValue({ id: 9, starred: 1 }); }); beforeEach(() => { - fileSvc.verifyTripAccess.mockReturnValue({ id: 5, user_id: 1 }); + canAccessTrip.mockReturnValue({ id: 5, user_id: 1 }); checkPermission.mockReturnValue(true); helperSvc.canAccessTrekPhoto.mockReturnValue(true); }); @@ -88,23 +98,30 @@ describe('Files + photos e2e (real auth guard + temp SQLite)', () => { expect((await request(server).get('/api/trips/5/files')).status).toBe(401); }); - it('200 list for an accessible trip', async () => { + it('200 list for an accessible trip (real SQL, formatted rows)', async () => { const res = await request(server).get('/api/trips/5/files').set('Cookie', sessionCookie(1)); expect(res.status).toBe(200); - expect(res.body).toEqual({ files: [{ id: 1, original_name: 'a.pdf' }] }); + expect(res.body.files).toHaveLength(2); + const first = res.body.files.find((f: { id: number }) => f.id === 1); + expect(first.original_name).toBe('a.pdf'); + expect(first.url).toBe('/api/trips/5/files/1/download'); + expect(first.linked_reservation_ids).toEqual([]); }); it('404 when the trip is not accessible', async () => { - fileSvc.verifyTripAccess.mockReturnValue(undefined); + canAccessTrip.mockReturnValue(undefined); const res = await request(server).get('/api/trips/5/files').set('Cookie', sessionCookie(1)); expect(res.status).toBe(404); expect(res.body).toEqual({ error: 'Trip not found' }); }); - it('200 toggling a star with permission', async () => { + it('200 toggling a star with permission (real UPDATE + re-select)', async () => { const res = await request(server).patch('/api/trips/5/files/9/star').set('Cookie', sessionCookie(1)); expect(res.status).toBe(200); - expect(res.body).toEqual({ file: { id: 9, starred: 1 } }); + expect(res.body.file.id).toBe(9); + expect(res.body.file.starred).toBe(1); + // put it back so the case is order-independent + db.prepare('UPDATE trip_files SET starred = 0 WHERE id = 9').run(); }); it('403 deleting without file_delete permission', async () => { @@ -115,7 +132,6 @@ describe('Files + photos e2e (real auth guard + temp SQLite)', () => { }); it('download is unguarded but enforces its own token auth (401 without one)', async () => { - fileSvc.authenticateDownload.mockReturnValue({ error: 'Authentication required', status: 401 }); const res = await request(server).get('/api/trips/5/files/9/download'); expect(res.status).toBe(401); expect(res.body).toEqual({ error: 'Authentication required' }); diff --git a/server/tests/e2e/journey.e2e.test.ts b/server/tests/e2e/journey.e2e.test.ts index 149861c66..78ab6f1ed 100644 --- a/server/tests/e2e/journey.e2e.test.ts +++ b/server/tests/e2e/journey.e2e.test.ts @@ -27,12 +27,10 @@ vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: ( const { isAddonEnabled } = vi.hoisted(() => ({ isAddonEnabled: vi.fn(() => true) })); vi.mock('../../src/services/adminService', () => ({ isAddonEnabled })); -vi.mock('../../src/services/fileService', () => ({ - getAllowedExtensions: () => '*', - MAX_VIDEO_SIZE: 500 * 1024 * 1024, - isVideoExtension: (ext: string) => ['mp4', 'm4v', 'webm', 'mov'].includes(String(ext).toLowerCase().replace(/^\./, '')), - isVideoMime: (m?: string) => !!m && m.startsWith('video/'), -})); +// The controller's pure helpers (isVideoExtension/isVideoMime/MAX_VIDEO_SIZE) +// now come from the real files.constants; only the request-time app_settings +// read is mocked, preserving the old '*'-allowlist semantics for the fixtures. +vi.mock('../../src/nest/files/files.bridge', () => ({ getAllowedExtensions: () => '*' })); vi.mock('../../src/services/memories/immichService', () => ({ uploadToImmich: vi.fn(), streamImmichAsset: vi.fn() })); vi.mock('../../src/services/memories/photoResolverService', () => ({ streamPhoto: vi.fn() })); diff --git a/server/tests/e2e/trips.e2e.test.ts b/server/tests/e2e/trips.e2e.test.ts index 42a455df2..a20c1e423 100644 --- a/server/tests/e2e/trips.e2e.test.ts +++ b/server/tests/e2e/trips.e2e.test.ts @@ -18,7 +18,8 @@ const { db } = vi.hoisted(() => { const tmp = new Database(':memory:'); tmp.exec('PRAGMA journal_mode = WAL'); tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL, - email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0);`); + email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0, + avatar TEXT);`); tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT);'); // bundle()'s todoItems now runs TodoService's real SQL (DI-injected, no mock). tmp.exec(`CREATE TABLE todo_items (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, @@ -34,6 +35,16 @@ const { db } = vi.hoisted(() => { created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); tmp.exec(`CREATE TABLE packing_item_recipients (item_id INTEGER NOT NULL, user_id INTEGER NOT NULL, PRIMARY KEY (item_id, user_id));`); + // bundle()'s files now runs FilesService's real SQL (DI-injected, no mock) — + // empty tables satisfy the FILE_SELECT joins and the file_links batch. + tmp.exec(`CREATE TABLE trip_files (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, + place_id INTEGER, reservation_id INTEGER, filename TEXT NOT NULL, original_name TEXT NOT NULL, + file_size INTEGER, mime_type TEXT, description TEXT, uploaded_by INTEGER, starred INTEGER DEFAULT 0, + deleted_at DATETIME, created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); + tmp.exec(`CREATE TABLE file_links (id INTEGER PRIMARY KEY AUTOINCREMENT, file_id INTEGER NOT NULL, + reservation_id INTEGER, assignment_id INTEGER, place_id INTEGER, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); + tmp.exec('CREATE TABLE IF NOT EXISTS reservations (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER, title TEXT);'); return { db: tmp }; }); @@ -62,7 +73,6 @@ vi.mock('../../src/services/dayService', () => ({ listDays: () => ({ days: [] }) vi.mock('../../src/services/placeService', () => ({ listPlaces: () => [] })); vi.mock('../../src/services/budgetService', () => ({ listBudgetItems: () => [] })); vi.mock('../../src/services/reservationService', () => ({ listReservations: () => [] })); -vi.mock('../../src/services/fileService', () => ({ listFiles: () => [] })); import { TripsModule } from '../../src/nest/trips/trips.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index c4e657a4b..501cace68 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -13,6 +13,7 @@ import { DayNotesService } from '../../src/nest/days/day-notes.service'; import { AssignmentsService } from '../../src/nest/assignments/assignments.service'; import { LlmConfigResolver } from '../../src/nest/llm-parse/llm-config.resolver'; import { SettingsService } from '../../src/nest/settings/settings.service'; +import { FilesService } from '../../src/nest/files/files.service'; /** * Hand-wired counterpart of the PluginsModule DI graph for no-Nest tests @@ -33,6 +34,7 @@ export function createHostDepsFactory(dbs: DatabaseService): PluginHostDepsFacto new AssignmentsService(dbs), new LlmConfigResolver(new SettingsService(dbs), dbs), dbs, + new FilesService(dbs), ); } diff --git a/server/tests/unit/nest/files.service.test.ts b/server/tests/unit/nest/files.service.test.ts index a65a1102f..f4cfaf8ae 100644 --- a/server/tests/unit/nest/files.service.test.ts +++ b/server/tests/unit/nest/files.service.test.ts @@ -1,134 +1,548 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import type { Request } from 'express'; +/** + * Unit tests for the DI-native FilesService — FILE-SVC-001 through FILE-SVC-036. + * The legacy services/fileService.ts never had a unit suite (behavior was pinned + * only by tests/integration/files.test.ts), so these cases are new: they pin the + * relocated SQL, the falsy-coercion defaults, the unlink-first delete semantics + * and the download-token auth over a real in-memory SQLite DB, plus the + * files.bridge delegation (inside the src/nest coverage gate). + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; +import path from 'path'; +import fs from 'fs'; -// Mock the side-effect dependencies the wrapper reaches into directly. -const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() })); -vi.mock('../../../src/websocket', () => ({ broadcast })); +// ── DB setup ────────────────────────────────────────────────────────────────── + +const { testDb, bareDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + // A second, schema-less DB for the getAllowedExtensions catch branch. + const bare = new Database(':memory:'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: (tripId: unknown, userId: number) => + db.prepare(` + SELECT t.id, t.user_id FROM trips t + LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? + WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) + `).get(userId, tripId, userId), + isOwner: (tripId: unknown, userId: number) => + !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), + }; + return { testDb: db, bareDb: bare, dbMock: mock }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-secret', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + updateJwtSecret: () => {}, +})); +vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn(() => true) })); vi.mock('../../../src/services/permissions', () => ({ checkPermission })); -const { svc } = vi.hoisted(() => ({ - svc: { - verifyTripAccess: vi.fn(), - authenticateDownload: vi.fn(), - resolveFilePath: vi.fn(), - listFiles: vi.fn(), - getFileById: vi.fn(), - getDeletedFile: vi.fn(), - createFile: vi.fn(), - updateFile: vi.fn(), - toggleStarred: vi.fn(), - softDeleteFile: vi.fn(), - restoreFile: vi.fn(), - permanentDeleteFile: vi.fn(), - emptyTrash: vi.fn(), - createFileLink: vi.fn(), - deleteFileLink: vi.fn(), - getFileLinks: vi.fn(), - }, -})); -vi.mock('../../../src/services/fileService', () => svc); +const { verifyJwtAndLoadUser } = vi.hoisted(() => ({ verifyJwtAndLoadUser: vi.fn() })); +vi.mock('../../../src/middleware/auth', () => ({ verifyJwtAndLoadUser })); +const { consumeEphemeralToken } = vi.hoisted(() => ({ consumeEphemeralToken: vi.fn() })); +vi.mock('../../../src/services/ephemeralTokens', () => ({ consumeEphemeralToken })); + +import type { Request } from 'express'; +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createTrip, addTripMember, createPlace, createReservation, createDay, createDayAssignment, setAppSetting } from '../../helpers/factories'; +import { DatabaseService } from '../../../src/nest/database/database.service'; import { FilesService } from '../../../src/nest/files/files.service'; -import type { User } from '../../../src/types'; +import { getAllowedExtensions as bridgeGetAllowedExtensions } from '../../../src/nest/files/files.bridge'; +import { + DEFAULT_ALLOWED_EXTENSIONS, + MAX_FILE_SIZE, + MAX_VIDEO_SIZE, + BLOCKED_EXTENSIONS, + filesDir, + isVideoMime, + isVideoExtension, +} from '../../../src/nest/files/files.constants'; +import type { TripFile, User } from '../../../src/types'; -function service() { - return new FilesService(); +const svc = new FilesService(new DatabaseService(testDb)); + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); + vi.clearAllMocks(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +afterAll(() => { + testDb.close(); + bareDb.close(); +}); + +function seedTrip() { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + return { user, trip }; } -beforeEach(() => vi.clearAllMocks()); +function makeFile(tripId: number, userId: number, overrides: Partial<{ filename: string; originalname: string; size: number; mimetype: string }> = {}, opts: Parameters[3] = {}) { + return svc.createFile( + tripId, + { filename: 'stored-name.pdf', originalname: 'visa.pdf', size: 1234, mimetype: 'application/pdf', ...overrides }, + userId, + opts + ); +} -describe('FilesService (thin wrapper around the legacy fileService)', () => { - it('verifyTripAccess delegates to the legacy service', () => { - svc.verifyTripAccess.mockReturnValue({ id: 5, user_id: 2 }); - expect(service().verifyTripAccess('5', 2)).toEqual({ id: 5, user_id: 2 }); - expect(svc.verifyTripAccess).toHaveBeenCalledWith('5', 2); +function req(parts: { cookie?: string; bearer?: string; token?: string }): Request { + return { + cookies: parts.cookie ? { trek_session: parts.cookie } : {}, + headers: parts.bearer ? { authorization: `Bearer ${parts.bearer}` } : {}, + query: parts.token ? { token: parts.token } : {}, + } as unknown as Request; +} + +// ── constants & pure helpers (files.constants.ts) ───────────────────────────── + +describe('files.constants', () => { + it('FILE-SVC-001: isVideoMime accepts video/* only', () => { + expect(isVideoMime('video/mp4')).toBe(true); + expect(isVideoMime('image/png')).toBe(false); + expect(isVideoMime(undefined)).toBe(false); + expect(isVideoMime(null)).toBe(false); + expect(isVideoMime('')).toBe(false); }); - it('can forwards the ownership flag when the user owns the trip', () => { - checkPermission.mockReturnValue(true); - const user = { id: 1, role: 'user' } as User; - expect(service().can('file_edit', { user_id: 1 } as never, user)).toBe(true); - expect(checkPermission).toHaveBeenCalledWith('file_edit', 'user', 1, 1, false); + it('FILE-SVC-002: isVideoExtension strips a leading dot and lowercases', () => { + expect(isVideoExtension('mp4')).toBe(true); + expect(isVideoExtension('.MP4')).toBe(true); + expect(isVideoExtension('.mov')).toBe(true); + expect(isVideoExtension('pdf')).toBe(false); + expect(isVideoExtension('svg')).toBe(false); }); - it('can marks the user as a guest when they do not own the trip', () => { - checkPermission.mockReturnValue(false); - const user = { id: 1, role: 'user' } as User; - expect(service().can('file_upload', { user_id: 2 } as never, user)).toBe(false); - expect(checkPermission).toHaveBeenCalledWith('file_upload', 'user', 2, 1, true); + it('FILE-SVC-003: blocklist, size caps and the default allowlist are pinned', () => { + expect(BLOCKED_EXTENSIONS).toHaveLength(25); + expect(BLOCKED_EXTENSIONS).toContain('.svg'); + expect(BLOCKED_EXTENSIONS).toContain('.exe'); + expect(BLOCKED_EXTENSIONS).toContain('.ps1'); + expect(MAX_FILE_SIZE).toBe(50 * 1024 * 1024); + expect(MAX_VIDEO_SIZE).toBe(500 * 1024 * 1024); + expect(DEFAULT_ALLOWED_EXTENSIONS).toBe('jpg,jpeg,png,gif,webp,heic,pdf,doc,docx,xls,xlsx,txt,csv,pkpass,pkpasses,md,markdown'); }); - it('broadcast forwards to the websocket helper', () => { - service().broadcast('5', 'file:created', { file: { id: 1 } }, 'sock'); - expect(broadcast).toHaveBeenCalledWith('5', 'file:created', { file: { id: 1 } }, 'sock'); + it('FILE-SVC-004: filesDir resolves to /uploads/files despite the deeper module location', () => { + const serverRoot = path.resolve(__dirname, '../../..'); + expect(path.resolve(filesDir)).toBe(path.join(serverRoot, 'uploads', 'files')); }); - it('authenticateDownload / resolveFilePath delegate', () => { - const req = { headers: {} } as Request; - svc.authenticateDownload.mockReturnValue({ userId: 7 }); - expect(service().authenticateDownload(req)).toEqual({ userId: 7 }); - expect(svc.authenticateDownload).toHaveBeenCalledWith(req); + it('FILE-SVC-005: resolveFilePath neutralizes traversal via basename and stays inside filesDir', () => { + const plain = svc.resolveFilePath('photo.jpg'); + expect(plain.resolved).toBe(path.resolve(path.join(filesDir, 'photo.jpg'))); + expect(plain.safe).toBe(true); - svc.resolveFilePath.mockReturnValue({ resolved: '/a/b.pdf', safe: true }); - expect(service().resolveFilePath('b.pdf')).toEqual({ resolved: '/a/b.pdf', safe: true }); - expect(svc.resolveFilePath).toHaveBeenCalledWith('b.pdf'); - }); - - it('the read helpers delegate', () => { - svc.listFiles.mockReturnValue([{ id: 1 }]); - expect(service().listFiles('5', true)).toEqual([{ id: 1 }]); - expect(svc.listFiles).toHaveBeenCalledWith('5', true); - - svc.getFileById.mockReturnValue({ id: 9 }); - expect(service().getFileById('9', '5')).toEqual({ id: 9 }); - expect(svc.getFileById).toHaveBeenCalledWith('9', '5'); - - svc.getDeletedFile.mockReturnValue({ id: 9 }); - expect(service().getDeletedFile('9', '5')).toEqual({ id: 9 }); - expect(svc.getDeletedFile).toHaveBeenCalledWith('9', '5'); - - svc.getFileLinks.mockReturnValue([{ id: 1 }]); - expect(service().getFileLinks('9')).toEqual([{ id: 1 }]); - expect(svc.getFileLinks).toHaveBeenCalledWith('9'); - }); - - it('the mutating helpers delegate', () => { - const file = { filename: 'a.pdf' } as Express.Multer.File; - svc.createFile.mockReturnValue({ id: 9 }); - expect(service().createFile('5', file, 1, { description: 'd' })).toEqual({ id: 9 }); - expect(svc.createFile).toHaveBeenCalledWith('5', file, 1, { description: 'd' }); - - svc.updateFile.mockReturnValue({ id: 9 }); - const current = { id: 9 } as never; - expect(service().updateFile('9', current, { description: 'x' })).toEqual({ id: 9 }); - expect(svc.updateFile).toHaveBeenCalledWith('9', current, { description: 'x' }); - - svc.toggleStarred.mockReturnValue({ id: 9, starred: 1 }); - expect(service().toggleStarred('9', 0)).toEqual({ id: 9, starred: 1 }); - expect(svc.toggleStarred).toHaveBeenCalledWith('9', 0); - - service().softDeleteFile('9'); - expect(svc.softDeleteFile).toHaveBeenCalledWith('9'); - - svc.restoreFile.mockReturnValue({ id: 9 }); - expect(service().restoreFile('9')).toEqual({ id: 9 }); - expect(svc.restoreFile).toHaveBeenCalledWith('9'); - - const trashed = { id: 9 } as never; - service().permanentDeleteFile(trashed); - expect(svc.permanentDeleteFile).toHaveBeenCalledWith(trashed); - - svc.emptyTrash.mockReturnValue(3); - expect(service().emptyTrash('5')).toBe(3); - expect(svc.emptyTrash).toHaveBeenCalledWith('5'); - - svc.createFileLink.mockReturnValue([{ id: 1 }]); - expect(service().createFileLink('9', { reservation_id: 2 })).toEqual([{ id: 1 }]); - expect(svc.createFileLink).toHaveBeenCalledWith('9', { reservation_id: 2 }); - - service().deleteFileLink('3', '9'); - expect(svc.deleteFileLink).toHaveBeenCalledWith('3', '9'); + const sneaky = svc.resolveFilePath('../../../etc/passwd'); + expect(sneaky.resolved).toBe(path.resolve(path.join(filesDir, 'passwd'))); + expect(sneaky.safe).toBe(true); + expect(sneaky.resolved.startsWith(path.resolve(filesDir))).toBe(true); + }); +}); + +// ── getAllowedExtensions ────────────────────────────────────────────────────── + +describe('getAllowedExtensions', () => { + it('FILE-SVC-006: returns the app_settings value when set', () => { + setAppSetting(testDb, 'allowed_file_types', 'pdf,txt'); + expect(svc.getAllowedExtensions()).toBe('pdf,txt'); + }); + + it('FILE-SVC-007: returns the default when the row is absent', () => { + expect(svc.getAllowedExtensions()).toBe(DEFAULT_ALLOWED_EXTENSIONS); + }); + + it('FILE-SVC-008: returns the default for an empty value (|| coercion, not ??)', () => { + setAppSetting(testDb, 'allowed_file_types', ''); + expect(svc.getAllowedExtensions()).toBe(DEFAULT_ALLOWED_EXTENSIONS); + }); + + it('FILE-SVC-009: returns the default when the query throws (no app_settings table)', () => { + const bareSvc = new FilesService(new DatabaseService(bareDb)); + expect(bareSvc.getAllowedExtensions()).toBe(DEFAULT_ALLOWED_EXTENSIONS); + }); +}); + +// ── trip-scoped reads ───────────────────────────────────────────────────────── + +describe('getFileById / getDeletedFile', () => { + it('FILE-SVC-010: getFileById is trip-scoped', () => { + const { user, trip } = seedTrip(); + const other = createTrip(testDb, user.id); + const file = makeFile(trip.id, user.id); + expect(svc.getFileById(file.id, trip.id)?.id).toBe(file.id); + expect(svc.getFileById(file.id, other.id)).toBeUndefined(); + }); + + it('FILE-SVC-011: getDeletedFile returns only soft-deleted rows', () => { + const { user, trip } = seedTrip(); + const file = makeFile(trip.id, user.id); + expect(svc.getDeletedFile(file.id, trip.id)).toBeUndefined(); + svc.softDeleteFile(file.id); + expect(svc.getDeletedFile(file.id, trip.id)?.id).toBe(file.id); + }); +}); + +describe('listFiles', () => { + it('FILE-SVC-012: active list excludes trash, is starred-first and carries the formatted shape', () => { + const { user, trip } = seedTrip(); + makeFile(trip.id, user.id, { originalname: 'plain.pdf' }); + const starred = makeFile(trip.id, user.id, { originalname: 'starred.pdf' }); + const trashed = makeFile(trip.id, user.id, { originalname: 'gone.pdf' }); + svc.toggleStarred(starred.id, 0); + svc.softDeleteFile(trashed.id); + + const files = svc.listFiles(trip.id, false) as Record[]; + expect(files.map((f) => f.id)).toHaveLength(2); + expect(files[0].id).toBe(starred.id); // ORDER BY f.starred DESC first + expect(files.map((f) => f.id)).not.toContain(trashed.id); + expect(files[0].url).toBe(`/api/trips/${trip.id}/files/${starred.id}/download`); + expect(files[0].uploaded_by_name).toBe(user.username); + expect(files[0]).toHaveProperty('uploaded_by_avatar'); + }); + + it('FILE-SVC-013: trash list returns only soft-deleted files', () => { + const { user, trip } = seedTrip(); + const kept = makeFile(trip.id, user.id); + const trashed = makeFile(trip.id, user.id); + svc.softDeleteFile(trashed.id); + + const trash = svc.listFiles(trip.id, true) as Record[]; + expect(trash.map((f) => f.id)).toEqual([trashed.id]); + expect(trash.map((f) => f.id)).not.toContain(kept.id); + }); + + it('FILE-SVC-014: batches file_links and filters null targets by truthiness', () => { + const { user, trip } = seedTrip(); + const reservation = createReservation(testDb, trip.id); + const place = createPlace(testDb, trip.id); + const linked = makeFile(trip.id, user.id); + const bare = makeFile(trip.id, user.id); + svc.createFileLink(linked.id, { reservation_id: reservation.id }); + svc.createFileLink(linked.id, { place_id: place.id }); + + const files = svc.listFiles(trip.id, false) as Record[]; + const linkedRow = files.find((f) => f.id === linked.id); + const bareRow = files.find((f) => f.id === bare.id); + expect(linkedRow.linked_reservation_ids).toEqual([reservation.id]); + expect(linkedRow.linked_place_ids).toEqual([place.id]); + expect(bareRow.linked_reservation_ids).toEqual([]); + expect(bareRow.linked_place_ids).toEqual([]); + + // The empty-trip guard skips the IN () batch entirely. + const empty = createTrip(testDb, user.id); + expect(svc.listFiles(empty.id, false)).toEqual([]); + }); +}); + +// ── createFile / updateFile / toggleStarred ─────────────────────────────────── + +describe('createFile', () => { + it('FILE-SVC-015: coerces falsy opts to NULL (|| null) and re-selects the formatted row', () => { + const { user, trip } = seedTrip(); + const file = makeFile(trip.id, user.id, {}, { place_id: '', reservation_id: undefined, description: '' }); + const row = testDb.prepare('SELECT * FROM trip_files WHERE id = ?').get(file.id) as Record; + expect(row.place_id).toBeNull(); + expect(row.reservation_id).toBeNull(); + expect(row.description).toBeNull(); + expect(file.url).toBe(`/api/trips/${trip.id}/files/${file.id}/download`); + expect((file as unknown as Record).uploaded_by_name).toBe(user.username); + }); + + it('FILE-SVC-016: stores the provided metadata and links the reservation title through FILE_SELECT', () => { + const { user, trip } = seedTrip(); + const reservation = createReservation(testDb, trip.id, { title: 'Night train' }); + const file = makeFile(trip.id, user.id, { originalname: 'ticket.pdf', size: 99, mimetype: 'application/pdf' }, { + reservation_id: String(reservation.id), + description: 'the booking', + }); + expect((file as unknown as Record).reservation_title).toBe('Night train'); + expect(file.description).toBe('the booking'); + expect(file.original_name).toBe('ticket.pdf'); + expect(file.file_size).toBe(99); + expect(file.uploaded_by).toBe(user.id); + }); +}); + +describe('updateFile', () => { + it('FILE-SVC-017: undefined fields keep the current values', () => { + const { user, trip } = seedTrip(); + const place = createPlace(testDb, trip.id); + const file = makeFile(trip.id, user.id, {}, { description: 'keep me', place_id: String(place.id) }); + const current = svc.getFileById(file.id, trip.id)!; + const updated = svc.updateFile(file.id, current, {}) as Record; + expect(updated.description).toBe('keep me'); + expect(updated.place_id).toBe(place.id); + }); + + it("FILE-SVC-018: '' clears every field (|| null) — description matches createFile's coercion", () => { + const { user, trip } = seedTrip(); + const place = createPlace(testDb, trip.id); + const reservation = createReservation(testDb, trip.id); + const file = makeFile(trip.id, user.id, {}, { description: 'old', place_id: String(place.id), reservation_id: String(reservation.id) }); + const current = svc.getFileById(file.id, trip.id)!; + const updated = svc.updateFile(file.id, current, { description: '', place_id: '', reservation_id: null }) as Record; + expect(updated.description).toBeNull(); // '' → NULL on update too (post-migration fix: symmetric with createFile) + expect(updated.place_id).toBeNull(); + expect(updated.reservation_id).toBeNull(); + }); +}); + +describe('toggleStarred / softDeleteFile / restoreFile', () => { + it('FILE-SVC-019: toggleStarred flips 0→1 and 1→0', () => { + const { user, trip } = seedTrip(); + const file = makeFile(trip.id, user.id); + const on = svc.toggleStarred(file.id, 0) as Record; + expect(on.starred).toBe(1); + const off = svc.toggleStarred(file.id, on.starred) as Record; + expect(off.starred).toBe(0); + }); + + it('FILE-SVC-020: softDeleteFile stamps deleted_at', () => { + const { user, trip } = seedTrip(); + const file = makeFile(trip.id, user.id); + svc.softDeleteFile(file.id); + const row = testDb.prepare('SELECT deleted_at FROM trip_files WHERE id = ?').get(file.id) as Record; + expect(row.deleted_at).not.toBeNull(); + }); + + it('FILE-SVC-021: restoreFile clears deleted_at and returns the formatted row', () => { + const { user, trip } = seedTrip(); + const file = makeFile(trip.id, user.id); + svc.softDeleteFile(file.id); + const restored = svc.restoreFile(file.id) as Record; + expect(restored.deleted_at).toBeNull(); + expect(restored.url).toBe(`/api/trips/${trip.id}/files/${file.id}/download`); + }); +}); + +// ── permanentDeleteFile / emptyTrash (unlink-first semantics) ───────────────── + +describe('permanentDeleteFile', () => { + it('FILE-SVC-022: removes the resolved path with force:true, then the DB row', async () => { + const { user, trip } = seedTrip(); + const file = makeFile(trip.id, user.id, { filename: 'on-disk.pdf' }); + const rm = vi.spyOn(fs.promises, 'rm').mockResolvedValue(undefined); + await svc.permanentDeleteFile(svc.getFileById(file.id, trip.id) as TripFile); + expect(rm).toHaveBeenCalledWith(svc.resolveFilePath('on-disk.pdf').resolved, { force: true }); + expect(svc.getFileById(file.id, trip.id)).toBeUndefined(); + }); + + it('FILE-SVC-023: an unlink failure logs [files], rethrows and keeps the DB row', async () => { + const { user, trip } = seedTrip(); + const file = makeFile(trip.id, user.id, { filename: 'stuck.pdf' }); + const boom = new Error('EACCES'); + vi.spyOn(fs.promises, 'rm').mockRejectedValue(boom); + const err = vi.spyOn(console, 'error').mockImplementation(() => {}); + await expect(svc.permanentDeleteFile(svc.getFileById(file.id, trip.id) as TripFile)).rejects.toThrow('EACCES'); + expect(err).toHaveBeenCalledWith('[files] unlink failed for stuck.pdf, keeping DB row:', boom); + expect(svc.getFileById(file.id, trip.id)).toBeDefined(); + }); +}); + +describe('emptyTrash', () => { + it('FILE-SVC-024: unlinks every trashed file and deletes their rows, returning the count', async () => { + const { user, trip } = seedTrip(); + const a = makeFile(trip.id, user.id, { filename: 'a.pdf' }); + const b = makeFile(trip.id, user.id, { filename: 'b.pdf' }); + const kept = makeFile(trip.id, user.id, { filename: 'kept.pdf' }); + svc.softDeleteFile(a.id); + svc.softDeleteFile(b.id); + vi.spyOn(fs.promises, 'rm').mockResolvedValue(undefined); + + await expect(svc.emptyTrash(trip.id)).resolves.toBe(2); + expect(svc.listFiles(trip.id, true)).toEqual([]); + expect(svc.getFileById(kept.id, trip.id)).toBeDefined(); + }); + + it('FILE-SVC-025: a partial unlink failure keeps the failed row, swallows the error and counts only successes', async () => { + const { user, trip } = seedTrip(); + const good = makeFile(trip.id, user.id, { filename: 'good.pdf' }); + const bad = makeFile(trip.id, user.id, { filename: 'bad.pdf' }); + svc.softDeleteFile(good.id); + svc.softDeleteFile(bad.id); + const boom = new Error('EBUSY'); + vi.spyOn(fs.promises, 'rm').mockImplementation((p) => + String(p).includes('bad.pdf') ? Promise.reject(boom) : Promise.resolve() + ); + const err = vi.spyOn(console, 'error').mockImplementation(() => {}); + + await expect(svc.emptyTrash(trip.id)).resolves.toBe(1); + expect(err).toHaveBeenCalledWith('[files] unlink failed for bad.pdf, keeping DB row:', boom); + expect(svc.getFileById(good.id, trip.id)).toBeUndefined(); + expect(svc.getDeletedFile(bad.id, trip.id)).toBeDefined(); + }); + + it('FILE-SVC-026: an empty trash resolves to 0 without touching the disk', async () => { + const { trip } = seedTrip(); + const rm = vi.spyOn(fs.promises, 'rm'); + await expect(svc.emptyTrash(trip.id)).resolves.toBe(0); + expect(rm).not.toHaveBeenCalled(); + }); +}); + +// ── findForeignLinkTarget ───────────────────────────────────────────────────── + +describe('findForeignLinkTarget', () => { + function twoTrips() { + const { user } = createUser(testDb); + const mine = createTrip(testDb, user.id); + const foreign = createTrip(testDb, user.id); + return { user, mine, foreign }; + } + + it('FILE-SVC-027: flags each foreign target kind and passes same-trip ids', () => { + const { mine, foreign } = twoTrips(); + const foreignRes = createReservation(testDb, foreign.id); + const foreignPlace = createPlace(testDb, foreign.id); + const foreignDay = createDay(testDb, foreign.id); + const foreignAssignment = createDayAssignment(testDb, foreignDay.id, createPlace(testDb, foreign.id).id); + const myRes = createReservation(testDb, mine.id); + + expect(svc.findForeignLinkTarget(mine.id, { reservation_id: foreignRes.id })).toBe('reservation_id'); + expect(svc.findForeignLinkTarget(mine.id, { place_id: foreignPlace.id })).toBe('place_id'); + expect(svc.findForeignLinkTarget(mine.id, { assignment_id: foreignAssignment.id })).toBe('assignment_id'); + expect(svc.findForeignLinkTarget(mine.id, { reservation_id: myRes.id })).toBeNull(); + }); + + it('FILE-SVC-028: falsy ids are skipped (they clear the link) and reservation is checked first', () => { + const { mine, foreign } = twoTrips(); + const foreignRes = createReservation(testDb, foreign.id); + const foreignPlace = createPlace(testDb, foreign.id); + + expect(svc.findForeignLinkTarget(mine.id, { reservation_id: 0, place_id: null, assignment_id: undefined })).toBeNull(); + expect(svc.findForeignLinkTarget(mine.id, { reservation_id: '' })).toBeNull(); + // Both foreign — the reservation check runs before the place check. + expect(svc.findForeignLinkTarget(mine.id, { reservation_id: foreignRes.id, place_id: foreignPlace.id })).toBe('reservation_id'); + }); +}); + +// ── file links ──────────────────────────────────────────────────────────────── + +describe('createFileLink / deleteFileLink / getFileLinks', () => { + it('FILE-SVC-029: inserts with || null coercion, dedupes via INSERT OR IGNORE and returns the re-select', () => { + const { user, trip } = seedTrip(); + const reservation = createReservation(testDb, trip.id, { title: 'Ferry' }); + const file = makeFile(trip.id, user.id); + + const links = svc.createFileLink(file.id, { reservation_id: reservation.id, place_id: '' }) as Record[]; + expect(links).toHaveLength(1); + expect(links[0].reservation_id).toBe(reservation.id); + expect(links[0].place_id).toBeNull(); + + const again = svc.createFileLink(file.id, { reservation_id: reservation.id }) as Record[]; + expect(again).toHaveLength(1); // UNIQUE(file_id, reservation_id) + OR IGNORE + + const hydrated = svc.getFileLinks(file.id) as Record[]; + expect(hydrated[0].reservation_title).toBe('Ferry'); + }); + + it('FILE-SVC-030: an insert error propagates (post-migration fix: no silent swallow)', () => { + const { user, trip } = seedTrip(); + const file = makeFile(trip.id, user.id); + expect(() => svc.createFileLink(file.id, { reservation_id: { bad: true } as never })).toThrow(); + }); + + it('FILE-SVC-031: deleteFileLink is scoped to (id AND file_id)', () => { + const { user, trip } = seedTrip(); + const reservation = createReservation(testDb, trip.id); + const file = makeFile(trip.id, user.id); + const other = makeFile(trip.id, user.id); + const [link] = svc.createFileLink(file.id, { reservation_id: reservation.id }) as Record[]; + + svc.deleteFileLink(link.id, other.id); // wrong file — no-op + expect(svc.getFileLinks(file.id)).toHaveLength(1); + svc.deleteFileLink(link.id, file.id); + expect(svc.getFileLinks(file.id)).toHaveLength(0); + }); +}); + +// ── download auth & glue ────────────────────────────────────────────────────── + +describe('authenticateDownload', () => { + it('FILE-SVC-032: a valid session cookie wins over a bearer token', () => { + verifyJwtAndLoadUser.mockReturnValue({ id: 42 }); + const result = svc.authenticateDownload(req({ cookie: 'cookie-jwt', bearer: 'bearer-jwt' })); + expect(result).toEqual({ userId: 42 }); + expect(verifyJwtAndLoadUser).toHaveBeenCalledWith('cookie-jwt'); + }); + + it('FILE-SVC-033: a bearer token is used when no cookie is present; invalid JWTs 401', () => { + verifyJwtAndLoadUser.mockReturnValue({ id: 7 }); + expect(svc.authenticateDownload(req({ bearer: 'bearer-jwt' }))).toEqual({ userId: 7 }); + expect(verifyJwtAndLoadUser).toHaveBeenCalledWith('bearer-jwt'); + + verifyJwtAndLoadUser.mockReturnValue(null); + expect(svc.authenticateDownload(req({ bearer: 'stale' }))).toEqual({ error: 'Invalid or expired token', status: 401 }); + }); + + it('FILE-SVC-034: a ?token= ephemeral token is consumed with the download purpose', () => { + consumeEphemeralToken.mockReturnValue(9); + expect(svc.authenticateDownload(req({ token: 'eph' }))).toEqual({ userId: 9 }); + expect(consumeEphemeralToken).toHaveBeenCalledWith('eph', 'download'); + + consumeEphemeralToken.mockReturnValue(null); + expect(svc.authenticateDownload(req({ token: 'spent' }))).toEqual({ error: 'Invalid or expired token', status: 401 }); + }); + + it('FILE-SVC-035: no credentials at all is a 401 Authentication required', () => { + expect(svc.authenticateDownload(req({}))).toEqual({ error: 'Authentication required', status: 401 }); + expect(verifyJwtAndLoadUser).not.toHaveBeenCalled(); + expect(consumeEphemeralToken).not.toHaveBeenCalled(); + }); +}); + +describe('verifyTripAccess / can / files.bridge', () => { + it('FILE-SVC-036: verifyTripAccess resolves owner and member, refuses strangers', () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const { user: stranger } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + + expect(svc.verifyTripAccess(trip.id, owner.id)?.id).toBe(trip.id); + expect(svc.verifyTripAccess(trip.id, member.id)).toBeDefined(); + expect(svc.verifyTripAccess(trip.id, stranger.id)).toBeFalsy(); + }); + + it('FILE-SVC-037: can() forwards to checkPermission with the shared-trip flag', () => { + const { user, trip } = seedTrip(); + const guest = { id: user.id + 1, role: 'user' } as unknown as User; + checkPermission.mockReturnValue(true); + const tripRow = { id: trip.id, user_id: user.id } as Record; + + expect(svc.can('file_edit', tripRow, guest)).toBe(true); + expect(checkPermission).toHaveBeenCalledWith('file_edit', 'user', user.id, guest.id, true); + + svc.can('file_upload', tripRow, { id: user.id, role: 'user' } as unknown as User); + expect(checkPermission).toHaveBeenLastCalledWith('file_upload', 'user', user.id, user.id, false); + }); + + it('FILE-SVC-038: files.bridge getAllowedExtensions delegates to the service over the shared db', () => { + setAppSetting(testDb, 'allowed_file_types', 'md,markdown'); + expect(bridgeGetAllowedExtensions()).toBe('md,markdown'); }); }); diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 5674fecf2..ef705b720 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -32,17 +32,18 @@ vi.mock('../../../src/services/budgetService', () => ({ rebaseTripCurrency: vi.fn(async () => {}), })); vi.mock('../../../src/services/reservationService', () => ({ listReservations: () => [] })); -vi.mock('../../../src/services/fileService', () => ({ listFiles: () => [] })); import { TripsService } from '../../../src/nest/trips/trips.service'; import type { TodoService } from '../../../src/nest/todo/todo.service'; import type { PackingService } from '../../../src/nest/packing/packing.service'; +import type { FilesService } from '../../../src/nest/files/files.service'; import { rebaseTripCurrency } from '../../../src/services/budgetService'; const todoStub = { listItems: () => [] } as unknown as TodoService; const packingListItems = vi.fn(() => []); const packingStub = { listItems: packingListItems } as unknown as PackingService; -function svc() { return new TripsService(new DatabaseService(dbConn), todoStub, packingStub); } +const filesStub = { listFiles: () => [] } as unknown as FilesService; +function svc() { return new TripsService(new DatabaseService(dbConn), todoStub, packingStub, filesStub); } beforeEach(() => vi.clearAllMocks()); describe('TripsService (wrapper delegation + bundle/copy/notify helpers)', () => { diff --git a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts index 799a3e6cd..d7a082878 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -4,7 +4,7 @@ * per-plugin data db is cached, a granted db:own call works through the wired * host, and trip broadcasts are force-namespaced to plugin:{id}:{event}. * DI-native domains (budget/reservations/tags/categories/todo/packing/ - * day-notes/assignments/oauth/llm-config) are constructor-injected stubs; + * day-notes/assignments/oauth/llm-config/files) are constructor-injected stubs; * legacy services/* domains stay path-mocked until their own DI migration lands. */ import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; @@ -177,10 +177,17 @@ const { testFilesDir } = vi.hoisted(() => { const pm = require('node:path') as typeof import('node:path'); return { testFilesDir: pm.join(osm.tmpdir(), 'trek-crh-files-test') }; }); -vi.mock('../../../src/services/fileService', () => ({ - listFiles: vi.fn((tid: number, trash: boolean) => [{ id: 2, trip_id: tid, trash }]), +// The load-time constants the factory imports from files.constants — mocked so +// createTripFile's mkdirSync/writeFileSync land in the tmpdir, never in the +// real server/uploads/files (same redirection the old fileService path mock did). +vi.mock('../../../src/nest/files/files.constants', () => ({ filesDir: testFilesDir, BLOCKED_EXTENSIONS: ['.exe', '.bat', '.sh'], +})); +// Files: injected stub since the fileService DI migration (same behaviors as +// the old path mock). +const filesStub = { + listFiles: vi.fn((tid: number, trash: boolean) => [{ id: 2, trip_id: tid, trash }]), createFile: vi.fn((tid: number, file: { filename: string; originalname: string; size: number }, uploadedBy: number) => ({ id: 130, trip_id: Number(tid), ...file, uploaded_by: uploadedBy })), createFileLink: vi.fn(() => [{ file_id: 130 }]), getFileById: vi.fn((id: number) => (Number(id) === 404 ? undefined : Number(id) === 500 ? { id: 500, filename: 'huge.mp4', original_name: 'huge.mp4', mime_type: 'video/mp4', file_size: 400 * 1024 * 1024, deleted_at: null } : { id: Number(id), filename: 'visa.pdf', original_name: 'visa.pdf', mime_type: 'application/pdf', file_size: 3, deleted_at: null, description: 'old', place_id: null, reservation_id: null })), @@ -190,7 +197,7 @@ vi.mock('../../../src/services/fileService', () => ({ // reservation_id/place_id 999 = belongs to another trip findForeignLinkTarget: vi.fn((_tid: number, opts: { reservation_id?: number | null; place_id?: number | null }) => (Number(opts.reservation_id) === 999 ? 'reservation_id' : Number(opts.place_id) === 999 ? 'place_id' : null)), -})); +} as unknown as FilesService; vi.mock('../../../src/services/collabService', () => ({ listNotes: vi.fn((tid: number) => [{ id: 1, trip_id: Number(tid), title: 'Note' }]), listPolls: vi.fn((tid: number) => [{ id: 2, trip_id: Number(tid), question: 'Q?' }]), @@ -301,12 +308,13 @@ import type { DayNotesService } from '../../../src/nest/days/day-notes.service'; import type { AssignmentsService } from '../../../src/nest/assignments/assignments.service'; import type { PluginOAuthService } from '../../../src/nest/plugins/plugin-oauth.service'; import type { LlmConfigResolver } from '../../../src/nest/llm-parse/llm-config.resolver'; +import type { FilesService } from '../../../src/nest/files/files.service'; import { DatabaseService } from '../../../src/nest/database/database.service'; // The factory under test, wired exactly like PluginsModule does — but with the // DI-native domain services replaced by the stubs above. The shim keeps the // ~45 historical call sites unchanged and supplies a default no-op router. -const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub, llmConfigStub, new DatabaseService(mockDb)); +const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub, llmConfigStub, new DatabaseService(mockDb), filesStub); const stubRouter: PluginCallRouter = { callPlugin: async () => undefined, emitPluginEvent: () => {} }; const createRealRpcHost = (id: string, granted: ReadonlySet, router: PluginCallRouter = stubRouter) => factory.create(id, granted, router);