diff --git a/client/src/components/Map/ReservationOverlay.tsx b/client/src/components/Map/ReservationOverlay.tsx index cbdf57d9..8db225f8 100644 --- a/client/src/components/Map/ReservationOverlay.tsx +++ b/client/src/components/Map/ReservationOverlay.tsx @@ -3,6 +3,7 @@ import { renderToStaticMarkup } from 'react-dom/server' import { Marker, Polyline, Tooltip, useMap, useMapEvents } from 'react-leaflet' import L from 'leaflet' import { Plane, Train, Ship, Car, Bus, Sailboat, Bike, CarTaxiFront, Route } from 'lucide-react' +import { escapeHtml } from '@trek/shared' import { useSettingsStore } from '../../store/settingsStore' import type { Reservation, ReservationEndpoint } from '../../types' @@ -42,7 +43,7 @@ function useEndpointPane() { function endpointIcon(type: TransportType, label: string | null): L.DivIcon { const { icon: IconCmp, color } = TYPE_META[type] const svg = renderToStaticMarkup(createElement(IconCmp, { size: 13, color: 'white', strokeWidth: 2.5 })) - const labelHtml = label ? `${label}` : '' + const labelHtml = label ? `${escapeHtml(label)}` : '' const estWidth = label ? Math.max(40, label.length * 6 + 28) : 26 return L.divIcon({ className: 'trek-endpoint-marker', @@ -53,7 +54,7 @@ function endpointIcon(type: TransportType, label: string | null): L.DivIcon { border:1.5px solid #fff;color:#fff; font-family:var(--font-system);font-size:11px;font-weight:600;letter-spacing:0.3px;line-height:1; box-sizing:border-box;height:22px;white-space:nowrap; - ">${svg}${labelHtml ? `${label}` : ''}`, + ">${svg}${labelHtml ? `${escapeHtml(label)}` : ''}`, iconSize: [estWidth, 22], iconAnchor: [estWidth / 2, 11], popupAnchor: [0, -11], @@ -172,8 +173,8 @@ function buildStatsHtml(color: string, mainLabel: string | null, subLabel: strin ) + 22 const hasBoth = !!mainLabel && !!subLabel const height = hasBoth ? 36 : 22 - const main = mainLabel ? `${mainLabel}` : '' - const sub = subLabel ? `${subLabel}` : '' + const main = mainLabel ? `${escapeHtml(mainLabel)}` : '' + const sub = subLabel ? `${escapeHtml(subLabel)}` : '' const html = `
${label}` : '' + const labelHtml = label ? `${escapeHtml(label)}` : '' return `
${mainLabel}` : '' - const sub = subLabel ? `${subLabel}` : '' + const main = mainLabel ? `${escapeHtml(mainLabel)}` : '' + const sub = subLabel ? `${escapeHtml(subLabel)}` : '' const html = `
world'; + await downloadJourneyBookPDF(journey); + const iframe = getIframe()!; + const html = iframe.srcdoc; + + // The script tag, image beacon and event handler are stripped from the story. + expect(html).not.toContain('): void { try { @@ -46,6 +48,24 @@ export function noAccess() { return { content: [{ type: 'text' as const, text: 'Trip not found or access denied.' }], isError: true }; } +export function permissionDenied() { + return { content: [{ type: 'text' as const, text: 'You do not have permission to perform this action on this trip.' }], isError: true }; +} + +/** + * RBAC gate for MCP tools, mirroring the checkPermission() calls the REST/Nest + * routes run. Call this after canAccessTrip() with the same action key the + * matching REST route uses. Returns true when the user may perform `action` + * on `tripId`. + */ +export function hasTripPermission(action: string, tripId: number | string, userId: number): boolean { + const trip = db.prepare('SELECT user_id FROM trips WHERE id = ?').get(tripId) as { user_id?: number } | undefined; + if (!trip) return false; + const userRow = db.prepare('SELECT role FROM users WHERE id = ?').get(userId) as { role?: string } | undefined; + const tripOwnerId = typeof trip.user_id === 'number' ? trip.user_id : null; + return checkPermission(action, userRow?.role ?? 'user', tripOwnerId, userId, tripOwnerId !== userId); +} + export function ok(data: unknown) { return { content: [{ type: 'text' as const, text: JSON.stringify(data, null, 2) }] }; } diff --git a/server/src/mcp/tools/assignments.ts b/server/src/mcp/tools/assignments.ts index 80e6cd6c..d964a7bd 100644 --- a/server/src/mcp/tools/assignments.ts +++ b/server/src/mcp/tools/assignments.ts @@ -13,7 +13,7 @@ import { getDay } from '../../services/dayService'; import { safeBroadcast, TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, + demoDenied, noAccess, ok, hasTripPermission, permissionDenied, } from './_shared'; import { canRead, canWrite } from '../scopes'; @@ -38,6 +38,7 @@ export function registerAssignmentTools(server: McpServer, userId: number, scope async ({ tripId, dayId, placeId, notes }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); if (!dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; if (!placeExists(placeId, tripId)) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; const assignment = createAssignment(dayId, placeId, notes || null); @@ -60,6 +61,7 @@ export function registerAssignmentTools(server: McpServer, userId: number, scope async ({ tripId, dayId, assignmentId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); if (!assignmentExistsInDay(assignmentId, dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Assignment not found.' }], isError: true }; deleteAssignment(assignmentId); @@ -83,6 +85,7 @@ export function registerAssignmentTools(server: McpServer, userId: number, scope async ({ tripId, assignmentId, place_time, end_time }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); const existing = getAssignmentForTrip(assignmentId, tripId); if (!existing) return { content: [{ type: 'text' as const, text: 'Assignment not found.' }], isError: true }; const assignment = updateTime( @@ -111,6 +114,7 @@ export function registerAssignmentTools(server: McpServer, userId: number, scope async ({ tripId, assignmentId, newDayId, oldDayId, orderIndex }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); if (!getAssignmentForTrip(assignmentId, tripId)) return { content: [{ type: 'text' as const, text: 'Assignment not found.' }], isError: true }; if (!getDay(newDayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; const result = moveAssignment(assignmentId, newDayId, orderIndex ?? 0, oldDayId); @@ -151,6 +155,7 @@ export function registerAssignmentTools(server: McpServer, userId: number, scope async ({ tripId, assignmentId, userIds }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); if (!getAssignmentForTrip(assignmentId, tripId)) return { content: [{ type: 'text' as const, text: 'Assignment not found.' }], isError: true }; const participants = setAssignmentParticipants(assignmentId, userIds); safeBroadcast(tripId, 'assignment:participants', { assignmentId, participants }); @@ -174,6 +179,7 @@ export function registerAssignmentTools(server: McpServer, userId: number, scope async ({ tripId, dayId, assignmentIds }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); if (!getDay(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; reorderAssignments(dayId, assignmentIds); safeBroadcast(tripId, 'assignment:reordered', { dayId, assignmentIds }); diff --git a/server/src/mcp/tools/budget.ts b/server/src/mcp/tools/budget.ts index 18736ff4..d9e8361c 100644 --- a/server/src/mcp/tools/budget.ts +++ b/server/src/mcp/tools/budget.ts @@ -10,7 +10,7 @@ import { import { safeBroadcast, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, + demoDenied, noAccess, ok, hasTripPermission, permissionDenied, } from './_shared'; import { canWrite } from '../scopes'; import { isAddonEnabled } from '../../services/adminService'; @@ -38,6 +38,7 @@ export function registerBudgetTools(server: McpServer, userId: number, scopes: s async ({ tripId, name, category, total_price, note }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('budget_edit', tripId, userId)) return permissionDenied(); const item = createBudgetItem(tripId, { category, name, total_price, note }); safeBroadcast(tripId, 'budget:created', { item }); return ok({ item }); @@ -57,6 +58,7 @@ export function registerBudgetTools(server: McpServer, userId: number, scopes: s async ({ tripId, itemId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('budget_edit', tripId, userId)) return permissionDenied(); const deleted = deleteBudgetItem(itemId, tripId); if (!deleted) return { content: [{ type: 'text' as const, text: 'Budget item not found.' }], isError: true }; safeBroadcast(tripId, 'budget:deleted', { itemId }); @@ -85,6 +87,7 @@ export function registerBudgetTools(server: McpServer, userId: number, scopes: s async ({ tripId, itemId, name, category, total_price, persons, days, note }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('budget_edit', tripId, userId)) return permissionDenied(); const item = updateBudgetItem(itemId, tripId, { name, category, total_price, persons, days, note }); if (!item) return { content: [{ type: 'text' as const, text: 'Budget item not found.' }], isError: true }; safeBroadcast(tripId, 'budget:updated', { item }); @@ -111,6 +114,7 @@ export function registerBudgetTools(server: McpServer, userId: number, scopes: s async ({ tripId, name, category, total_price, note, userIds }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('budget_edit', tripId, userId)) return permissionDenied(); const hasMembers = userIds && userIds.length > 0; try { const run = db.transaction(() => { @@ -144,6 +148,7 @@ export function registerBudgetTools(server: McpServer, userId: number, scopes: s async ({ tripId, itemId, userIds }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('budget_edit', tripId, userId)) return permissionDenied(); const item = updateBudgetMembers(itemId, tripId, userIds); safeBroadcast(tripId, 'budget:members-updated', { item }); return ok({ item }); @@ -165,7 +170,8 @@ export function registerBudgetTools(server: McpServer, userId: number, scopes: s async ({ tripId, itemId, memberId, paid }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); - const member = toggleMemberPaid(itemId, memberId, paid); + if (!hasTripPermission('budget_edit', tripId, userId)) return permissionDenied(); + const member = toggleMemberPaid(itemId, tripId, memberId, paid); safeBroadcast(tripId, 'budget:member-paid-updated', { itemId, member }); return ok({ member }); } diff --git a/server/src/mcp/tools/collab.ts b/server/src/mcp/tools/collab.ts index 0a827b48..64a2ed93 100644 --- a/server/src/mcp/tools/collab.ts +++ b/server/src/mcp/tools/collab.ts @@ -12,7 +12,7 @@ import { ADDON_IDS } from '../../addons'; import { safeBroadcast, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, TOOL_ANNOTATIONS_READONLY, - demoDenied, noAccess, ok, + demoDenied, noAccess, ok, hasTripPermission, permissionDenied, } from './_shared'; import { canRead, canWrite } from '../scopes'; @@ -43,6 +43,7 @@ export function registerCollabTools(server: McpServer, userId: number, scopes: s async ({ tripId, title, content, category, color, pinned }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('collab_edit', tripId, userId)) return permissionDenied(); const note = createCollabNote(tripId, userId, { title, content, category, color, pinned }); safeBroadcast(tripId, 'collab:note:created', { note }); return ok({ note }); @@ -67,6 +68,7 @@ export function registerCollabTools(server: McpServer, userId: number, scopes: s async ({ tripId, noteId, title, content, category, color, pinned }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('collab_edit', tripId, userId)) return permissionDenied(); const note = updateCollabNote(tripId, noteId, { title, content, category, color, pinned }); if (!note) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; safeBroadcast(tripId, 'collab:note:updated', { note }); @@ -87,6 +89,7 @@ export function registerCollabTools(server: McpServer, userId: number, scopes: s async ({ tripId, noteId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('collab_edit', tripId, userId)) return permissionDenied(); const deleted = deleteCollabNote(tripId, noteId); if (!deleted) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; safeBroadcast(tripId, 'collab:note:deleted', { noteId }); @@ -128,6 +131,7 @@ export function registerCollabTools(server: McpServer, userId: number, scopes: s async ({ tripId, question, options, multiple, deadline }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('collab_edit', tripId, userId)) return permissionDenied(); const poll = createPoll(tripId, userId, { question, options, multiple, deadline }); safeBroadcast(tripId, 'collab:poll:created', { poll }); return ok({ poll }); @@ -147,6 +151,7 @@ export function registerCollabTools(server: McpServer, userId: number, scopes: s }, async ({ tripId, pollId, optionIndex }) => { if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('collab_edit', tripId, userId)) return permissionDenied(); const result = votePoll(tripId, pollId, userId, optionIndex); if (result.error) return { content: [{ type: 'text' as const, text: result.error }], isError: true }; safeBroadcast(tripId, 'collab:poll:voted', { poll: result.poll }); @@ -167,6 +172,7 @@ export function registerCollabTools(server: McpServer, userId: number, scopes: s async ({ tripId, pollId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('collab_edit', tripId, userId)) return permissionDenied(); const poll = closePoll(tripId, pollId); if (!poll) return { content: [{ type: 'text' as const, text: 'Poll not found.' }], isError: true }; safeBroadcast(tripId, 'collab:poll:closed', { poll }); @@ -187,6 +193,7 @@ export function registerCollabTools(server: McpServer, userId: number, scopes: s async ({ tripId, pollId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('collab_edit', tripId, userId)) return permissionDenied(); const deleted = deletePoll(tripId, pollId); if (!deleted) return { content: [{ type: 'text' as const, text: 'Poll not found.' }], isError: true }; safeBroadcast(tripId, 'collab:poll:deleted', { pollId }); @@ -225,6 +232,7 @@ export function registerCollabTools(server: McpServer, userId: number, scopes: s async ({ tripId, text, replyTo }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('collab_edit', tripId, userId)) return permissionDenied(); const result = createMessage(tripId, userId, text, replyTo ?? null); if (result.error) return { content: [{ type: 'text' as const, text: result.error }], isError: true }; safeBroadcast(tripId, 'collab:message:created', { message: result.message }); @@ -245,6 +253,7 @@ export function registerCollabTools(server: McpServer, userId: number, scopes: s async ({ tripId, messageId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('collab_edit', tripId, userId)) return permissionDenied(); const result = deleteMessage(tripId, messageId, userId); if (result.error) return { content: [{ type: 'text' as const, text: result.error }], isError: true }; safeBroadcast(tripId, 'collab:message:deleted', { messageId, username: result.username }); @@ -266,6 +275,7 @@ export function registerCollabTools(server: McpServer, userId: number, scopes: s async ({ tripId, messageId, emoji }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('collab_edit', tripId, userId)) return permissionDenied(); const result = addOrRemoveReaction(messageId, tripId, userId, emoji); if (!result.found) return { content: [{ type: 'text' as const, text: 'Message not found.' }], isError: true }; safeBroadcast(tripId, 'collab:message:reacted', { messageId, reactions: result.reactions }); diff --git a/server/src/mcp/tools/days.ts b/server/src/mcp/tools/days.ts index 83ff881c..0e479ca3 100644 --- a/server/src/mcp/tools/days.ts +++ b/server/src/mcp/tools/days.ts @@ -15,7 +15,7 @@ import { import { safeBroadcast, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, + demoDenied, noAccess, ok, hasTripPermission, permissionDenied, } from './_shared'; import { canWrite } from '../scopes'; @@ -38,6 +38,7 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri async ({ tripId, dayId, title }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); const current = getDay(dayId, tripId); if (!current) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; const updated = updateDay(dayId, current, title !== undefined ? { title } : {}); @@ -60,6 +61,7 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri async ({ tripId, date, notes }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); const day = createDay(tripId, date, notes); safeBroadcast(tripId, 'day:created', { day }); return ok({ day }); @@ -79,6 +81,7 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri async ({ tripId, dayId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); if (!getDay(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; deleteDay(dayId); safeBroadcast(tripId, 'day:deleted', { id: dayId }); @@ -105,6 +108,7 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri async ({ tripId, place_id, start_day_id, end_day_id, check_in, check_out, confirmation, notes }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); const errors = validateAccommodationRefs(tripId, place_id, start_day_id, end_day_id); if (errors.length > 0) return { content: [{ type: 'text' as const, text: errors.map(e => e.message).join(', ') }], isError: true }; const accommodation = createAccommodation(tripId, { place_id, start_day_id, end_day_id, check_in, check_out, confirmation, notes }); @@ -144,6 +148,7 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri async ({ tripId, name, description, lat, lng, address, category_id, google_place_id, osm_id, place_notes, website, phone, start_day_id, end_day_id, check_in, check_out, confirmation, accommodation_notes, price, currency }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); const dayErrors = validateAccommodationRefs(tripId, undefined, start_day_id, end_day_id); if (dayErrors.length > 0) return { content: [{ type: 'text' as const, text: dayErrors.map(e => e.message).join(', ') }], isError: true }; try { @@ -182,6 +187,7 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri async ({ tripId, accommodationId, place_id, start_day_id, end_day_id, check_in, check_out, confirmation, notes }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); const existing = getAccommodation(accommodationId, tripId); if (!existing) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true }; const accommodation = updateAccommodation(accommodationId, existing, { place_id, start_day_id, end_day_id, check_in, check_out, confirmation, notes }); @@ -203,6 +209,7 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri async ({ tripId, accommodationId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); if (!getAccommodation(accommodationId, tripId)) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true }; const { linkedReservationId } = deleteAccommodation(accommodationId); safeBroadcast(tripId, 'accommodation:deleted', { id: accommodationId, linkedReservationId }); @@ -228,6 +235,7 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri async ({ tripId, dayId, text, time, icon }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); if (!dayNoteExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; const note = createDayNote(dayId, tripId, text, time, icon); safeBroadcast(tripId, 'dayNote:created', { dayId, note }); @@ -252,6 +260,7 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri async ({ tripId, dayId, noteId, text, time, icon }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); const existing = getDayNote(noteId, dayId, tripId); if (!existing) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; const note = updateDayNote(noteId, existing, { text, time: time !== undefined ? time : undefined, icon }); @@ -274,6 +283,7 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri async ({ tripId, dayId, noteId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); const note = getDayNote(noteId, dayId, tripId); if (!note) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; deleteDayNote(noteId); diff --git a/server/src/mcp/tools/packing.ts b/server/src/mcp/tools/packing.ts index 981ed902..a3245bd8 100644 --- a/server/src/mcp/tools/packing.ts +++ b/server/src/mcp/tools/packing.ts @@ -14,7 +14,7 @@ import { import { safeBroadcast, TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, + demoDenied, noAccess, ok, hasTripPermission, permissionDenied, } from './_shared'; import { canRead, canWrite } from '../scopes'; import { isAddonEnabled } from '../../services/adminService'; @@ -42,6 +42,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, name, category }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const item = createPackingItem(tripId, { name, category: category || 'General' }); safeBroadcast(tripId, 'packing:created', { item }); return ok({ item }); @@ -62,6 +63,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, itemId, checked }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const item = updatePackingItem(tripId, itemId, { checked: checked ? 1 : 0 }, ['checked']); if (!item) return { content: [{ type: 'text' as const, text: 'Packing item not found.' }], isError: true }; safeBroadcast(tripId, 'packing:updated', { item }); @@ -82,6 +84,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, itemId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const deleted = deletePackingItem(tripId, itemId); if (!deleted) return { content: [{ type: 'text' as const, text: 'Packing item not found.' }], isError: true }; safeBroadcast(tripId, 'packing:deleted', { itemId }); @@ -106,6 +109,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, itemId, name, category }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const bodyKeys = ['name', 'category'].filter(k => k === 'name' ? name !== undefined : category !== undefined); const item = updatePackingItem(tripId, itemId, { name, category }, bodyKeys); if (!item) return { content: [{ type: 'text' as const, text: 'Packing item not found.' }], isError: true }; @@ -129,6 +133,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, orderedIds }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); reorderPackingItems(tripId, orderedIds); safeBroadcast(tripId, 'packing:reordered', { orderedIds }); return ok({ success: true }); @@ -165,6 +170,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, name, color }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const bag = createBag(tripId, { name, color }); safeBroadcast(tripId, 'packing:bag-created', { bag }); return ok({ bag }); @@ -186,6 +192,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, bagId, name, color }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const fields: Record = {}; const bodyKeys: string[] = []; if (name !== undefined) { fields.name = name; bodyKeys.push('name'); } @@ -209,6 +216,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, bagId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); deleteBag(tripId, bagId); safeBroadcast(tripId, 'packing:bag-deleted', { id: bagId }); return ok({ success: true }); @@ -229,6 +237,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, bagId, userIds }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); setBagMembers(tripId, bagId, userIds); safeBroadcast(tripId, 'packing:bag-members-updated', { bagId, userIds }); return ok({ success: true }); @@ -265,6 +274,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, categoryName, userIds }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); updatePackingCategoryAssignees(tripId, categoryName, userIds); safeBroadcast(tripId, 'packing:assignees', { categoryName, userIds }); return ok({ success: true }); @@ -284,6 +294,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, templateId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const applied = applyTemplate(tripId, templateId); if (applied === null) return { content: [{ type: 'text' as const, text: 'Template not found.' }], isError: true }; safeBroadcast(tripId, 'packing:template-applied', { templateId }); @@ -304,6 +315,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, templateName }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); saveAsTemplate(tripId, userId, templateName); return ok({ success: true }); } @@ -326,6 +338,7 @@ export function registerPackingTools(server: McpServer, userId: number, scopes: async ({ tripId, items }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); bulkImport(tripId, items); safeBroadcast(tripId, 'packing:updated', {}); return ok({ success: true, count: items.length }); diff --git a/server/src/mcp/tools/places.ts b/server/src/mcp/tools/places.ts index a5bdcfe4..1578b55a 100644 --- a/server/src/mcp/tools/places.ts +++ b/server/src/mcp/tools/places.ts @@ -10,7 +10,7 @@ import { searchPlaces } from '../../services/mapsService'; import { safeBroadcast, TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, + demoDenied, noAccess, ok, hasTripPermission, permissionDenied, } from './_shared'; import { canRead, canWrite } from '../scopes'; @@ -45,6 +45,7 @@ export function registerPlaceTools(server: McpServer, userId: number, scopes: st async ({ tripId, name, description, lat, lng, address, category_id, google_place_id, osm_id, notes, website, phone, price, currency }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('place_edit', tripId, userId)) return permissionDenied(); const place = createPlace(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, osm_id, notes, website, phone, price, currency }); safeBroadcast(tripId, 'place:created', { place }); return ok({ place }); @@ -78,6 +79,7 @@ export function registerPlaceTools(server: McpServer, userId: number, scopes: st async ({ tripId, dayId, name, description, lat, lng, address, category_id, google_place_id, osm_id, place_notes, website, phone, assignment_notes, price, currency }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('place_edit', tripId, userId)) return permissionDenied(); if (!dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; try { const run = db.transaction(() => { @@ -125,6 +127,7 @@ export function registerPlaceTools(server: McpServer, userId: number, scopes: st async ({ tripId, placeId, name, description, lat, lng, address, category_id, price, currency, place_time, end_time, duration_minutes, notes, website, phone, transport_mode, osm_id, google_place_id }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('place_edit', tripId, userId)) return permissionDenied(); const place = updatePlace(String(tripId), String(placeId), { name, description, lat, lng, address, category_id, price, currency, place_time, end_time, duration_minutes, notes, website, phone, transport_mode, osm_id, google_place_id }); if (!place) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; safeBroadcast(tripId, 'place:updated', { place }); @@ -145,6 +148,7 @@ export function registerPlaceTools(server: McpServer, userId: number, scopes: st async ({ tripId, placeId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('place_edit', tripId, userId)) return permissionDenied(); const deleted = deletePlace(String(tripId), String(placeId)); if (!deleted) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; safeBroadcast(tripId, 'place:deleted', { placeId }); @@ -222,6 +226,7 @@ export function registerPlaceTools(server: McpServer, userId: number, scopes: st async ({ tripId, url, source }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('place_edit', tripId, userId)) return permissionDenied(); const result = source === 'google-list' ? await importGoogleList(String(tripId), url) @@ -251,6 +256,7 @@ export function registerPlaceTools(server: McpServer, userId: number, scopes: st async ({ tripId, placeIds }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('place_edit', tripId, userId)) return permissionDenied(); const deleted = deletePlacesMany(String(tripId), placeIds); for (const id of deleted) { diff --git a/server/src/mcp/tools/reservations.ts b/server/src/mcp/tools/reservations.ts index 9c8825fb..68ff20e4 100644 --- a/server/src/mcp/tools/reservations.ts +++ b/server/src/mcp/tools/reservations.ts @@ -12,7 +12,7 @@ import { placeExists, getAssignmentForTrip } from '../../services/assignmentServ import { safeBroadcast, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, + demoDenied, noAccess, ok, hasTripPermission, permissionDenied, } from './_shared'; import { canWrite } from '../scopes'; @@ -47,6 +47,7 @@ export function registerReservationTools(server: McpServer, userId: number, scop async ({ tripId, title, type, reservation_time, location, confirmation_number, notes, day_id, place_id, start_day_id, end_day_id, check_in, check_out, assignment_id, price, budget_category }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); // Validate that all referenced IDs belong to this trip if (day_id && !getDay(day_id, tripId)) @@ -113,6 +114,7 @@ export function registerReservationTools(server: McpServer, userId: number, scop async ({ tripId, reservationId, title, type, reservation_time, location, confirmation_number, notes, status, place_id, assignment_id }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); const existing = getReservation(reservationId, tripId); if (!existing) return { content: [{ type: 'text' as const, text: 'Reservation not found.' }], isError: true }; @@ -144,6 +146,7 @@ export function registerReservationTools(server: McpServer, userId: number, scop async ({ tripId, reservationId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); const { deleted, accommodationDeleted } = deleteReservation(reservationId, tripId); if (!deleted) return { content: [{ type: 'text' as const, text: 'Reservation not found.' }], isError: true }; if (accommodationDeleted) { @@ -171,6 +174,7 @@ export function registerReservationTools(server: McpServer, userId: number, scop async ({ tripId, positions, dayId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); updateReservationPositions(tripId, positions, dayId); safeBroadcast(tripId, 'reservation:positions', { positions, dayId }); return ok({ success: true }); @@ -195,6 +199,7 @@ export function registerReservationTools(server: McpServer, userId: number, scop async ({ tripId, reservationId, place_id, start_day_id, end_day_id, check_in, check_out }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); const current = getReservation(reservationId, tripId); if (!current) return { content: [{ type: 'text' as const, text: 'Reservation not found.' }], isError: true }; if (current.type !== 'hotel') return { content: [{ type: 'text' as const, text: 'Reservation is not of type hotel.' }], isError: true }; diff --git a/server/src/mcp/tools/todos.ts b/server/src/mcp/tools/todos.ts index 38838c91..b7a164e3 100644 --- a/server/src/mcp/tools/todos.ts +++ b/server/src/mcp/tools/todos.ts @@ -10,7 +10,7 @@ import { import { safeBroadcast, TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, + demoDenied, noAccess, ok, hasTripPermission, permissionDenied, } from './_shared'; import { canRead, canWrite } from '../scopes'; import { isAddonEnabled } from '../../services/adminService'; @@ -58,6 +58,7 @@ export function registerTodoTools(server: McpServer, userId: number, scopes: str async ({ tripId, name, category, due_date, description, assigned_user_id, priority }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const item = createTodoItem(tripId, { name, category, due_date, description, assigned_user_id, priority }); safeBroadcast(tripId, 'todo:created', { item }); return ok({ item }); @@ -83,6 +84,7 @@ export function registerTodoTools(server: McpServer, userId: number, scopes: str async ({ tripId, itemId, name, category, due_date, description, assigned_user_id, priority }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); // Build bodyKeys to signal which nullable fields were explicitly provided const bodyKeys: string[] = []; if (due_date !== undefined) bodyKeys.push('due_date'); @@ -110,6 +112,7 @@ export function registerTodoTools(server: McpServer, userId: number, scopes: str async ({ tripId, itemId, checked }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const item = updateTodoItem(tripId, itemId, { checked: checked ? 1 : 0 }, []); if (!item) return { content: [{ type: 'text' as const, text: 'To-do item not found.' }], isError: true }; safeBroadcast(tripId, 'todo:updated', { item }); @@ -130,6 +133,7 @@ export function registerTodoTools(server: McpServer, userId: number, scopes: str async ({ tripId, itemId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const deleted = deleteTodoItem(tripId, itemId); if (!deleted) return { content: [{ type: 'text' as const, text: 'To-do item not found.' }], isError: true }; safeBroadcast(tripId, 'todo:deleted', { itemId }); @@ -150,6 +154,7 @@ export function registerTodoTools(server: McpServer, userId: number, scopes: str async ({ tripId, orderedIds }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); reorderTodoItems(tripId, orderedIds); return ok({ success: true }); } @@ -185,6 +190,7 @@ export function registerTodoTools(server: McpServer, userId: number, scopes: str async ({ tripId, categoryName, userIds }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); const assignees = updateTodoCategoryAssignees(tripId, categoryName, userIds); safeBroadcast(tripId, 'todo:assignees', { category: categoryName, assignees }); return ok({ assignees }); diff --git a/server/src/mcp/tools/transports.ts b/server/src/mcp/tools/transports.ts index d2cf022e..578566c8 100644 --- a/server/src/mcp/tools/transports.ts +++ b/server/src/mcp/tools/transports.ts @@ -9,7 +9,7 @@ import { linkBudgetItemToReservation } from '../../services/budgetService'; import { getDay } from '../../services/dayService'; import { safeBroadcast, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - TOOL_ANNOTATIONS_WRITE, demoDenied, noAccess, ok, + TOOL_ANNOTATIONS_WRITE, demoDenied, noAccess, ok, hasTripPermission, permissionDenied, } from './_shared'; import { canWrite } from '../scopes'; @@ -56,6 +56,7 @@ export function registerTransportTools(server: McpServer, userId: number, scopes async ({ tripId, type, title, status, start_day_id, end_day_id, reservation_time, reservation_end_time, confirmation_number, notes, metadata, endpoints, needs_review, price, budget_category }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); if (start_day_id && !getDay(start_day_id, tripId)) return { content: [{ type: 'text' as const, text: 'start_day_id does not belong to this trip.' }], isError: true }; @@ -120,6 +121,7 @@ export function registerTransportTools(server: McpServer, userId: number, scopes async ({ tripId, reservationId, type, title, status, start_day_id, end_day_id, reservation_time, reservation_end_time, confirmation_number, notes, metadata, endpoints, needs_review }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); const existing = getReservation(reservationId, tripId); if (!existing) return { content: [{ type: 'text' as const, text: 'Transport not found.' }], isError: true }; @@ -165,6 +167,7 @@ export function registerTransportTools(server: McpServer, userId: number, scopes async ({ tripId, reservationId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); const { deleted } = deleteReservation(reservationId, tripId); if (!deleted) return { content: [{ type: 'text' as const, text: 'Transport not found.' }], isError: true }; safeBroadcast(tripId, 'reservation:deleted', { reservationId }); diff --git a/server/src/mcp/tools/trips.ts b/server/src/mcp/tools/trips.ts index e2d8aaca..a748e536 100644 --- a/server/src/mcp/tools/trips.ts +++ b/server/src/mcp/tools/trips.ts @@ -22,7 +22,7 @@ import { safeBroadcast, MAX_MCP_TRIP_DAYS, TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, + demoDenied, noAccess, ok, hasTripPermission, permissionDenied, } from './_shared'; import { canRead, canReadTrips, canWrite, canDeleteTrips, canShareTrips } from '../scopes'; @@ -84,6 +84,7 @@ export function registerTripTools(server: McpServer, userId: number, scopes: str async ({ tripId, title, description, start_date, end_date, currency }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('trip_edit', tripId, userId)) return permissionDenied(); if (start_date) { const d = new Date(start_date + 'T00:00:00Z'); if (isNaN(d.getTime()) || d.toISOString().slice(0, 10) !== start_date) @@ -321,6 +322,8 @@ export function registerTripTools(server: McpServer, userId: number, scopes: str annotations: TOOL_ANNOTATIONS_READONLY, }, async ({ tripId }) => { + // Read parity with the REST route GET /api/trips/:tripId/share-link, which + // only requires trip membership (share_manage gates create/delete, not read). if (!canAccessTrip(tripId, userId)) return noAccess(); const link = getShareLink(String(tripId)); return ok({ link }); @@ -344,6 +347,7 @@ export function registerTripTools(server: McpServer, userId: number, scopes: str async ({ tripId, share_map, share_bookings, share_packing, share_budget, share_collab }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('share_manage', tripId, userId)) return permissionDenied(); const { token, created } = createOrUpdateShareLink(String(tripId), userId, { share_map: share_map ?? true, share_bookings: share_bookings ?? true, @@ -367,6 +371,7 @@ export function registerTripTools(server: McpServer, userId: number, scopes: str async ({ tripId }) => { if (isDemoUser(userId)) return demoDenied(); if (!canAccessTrip(tripId, userId)) return noAccess(); + if (!hasTripPermission('share_manage', tripId, userId)) return permissionDenied(); deleteShareLink(String(tripId)); return ok({ success: true }); } diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts index 18be7236..6631a452 100644 --- a/server/src/middleware/auth.ts +++ b/server/src/middleware/auth.ts @@ -27,7 +27,11 @@ export function extractToken(req: Request): string | null { */ export function verifyJwtAndLoadUser(token: string): User | null { try { - const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number; pv?: number }; + const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number; pv?: number; purpose?: string }; + // Purpose-scoped tokens (e.g. the short-lived mfa_login token) share this + // secret but are not full session tokens — only their dedicated endpoint + // may accept them, so reject any token carrying a purpose claim here. + if (decoded.purpose) return null; const row = db.prepare( 'SELECT id, username, email, role, password_version FROM users WHERE id = ?' ).get(decoded.id) as (User & { password_version?: number }) | undefined; diff --git a/server/src/middleware/globalMiddleware.ts b/server/src/middleware/globalMiddleware.ts index 5aa02d49..9a9a55de 100644 --- a/server/src/middleware/globalMiddleware.ts +++ b/server/src/middleware/globalMiddleware.ts @@ -107,6 +107,9 @@ export function applyGlobalMiddleware( objectSrc: ["'none'"], frameSrc: ["'none'"], frameAncestors: ["'self'"], + // Restrict
submission targets (form-action has no default-src + // fallback, so it must be set explicitly). + formAction: ["'self'"], upgradeInsecureRequests: shouldForceHttps ? [] : null } }, diff --git a/server/src/nest/auth/auth.controller.ts b/server/src/nest/auth/auth.controller.ts index b0edd9bc..5da251fc 100644 --- a/server/src/nest/auth/auth.controller.ts +++ b/server/src/nest/auth/auth.controller.ts @@ -9,13 +9,14 @@ import { Post, Put, Req, + Res, UploadedFile, UseGuards, UseInterceptors, } from '@nestjs/common'; import { FileInterceptor } from '@nestjs/platform-express'; import { diskStorage } from 'multer'; -import type { Request } from 'express'; +import type { Request, Response } from 'express'; import path from 'path'; import fs from 'fs'; import { v4 as uuid } from 'uuid'; @@ -76,12 +77,15 @@ export class AuthController { } @Put('me/password') - changePassword(@CurrentUser() user: User, @Body() body: unknown, @Req() req: Request) { + changePassword(@CurrentUser() user: User, @Body() body: unknown, @Req() req: Request, @Res({ passthrough: true }) res: Response) { this.limit('login', req, 5); const result = this.auth.changePassword(user.id, user.email, body); if (result.error) { throw new HttpException({ error: result.error }, result.status!); } + // Refresh this device's cookie with the new password_version so the user + // stays logged in here while all other sessions are invalidated. + if (result.token) this.auth.setAuthCookie(res, result.token, req); writeAudit({ userId: user.id, action: 'user.password_change', ip: getClientIp(req) }); return { success: true }; } diff --git a/server/src/nest/budget/budget.controller.ts b/server/src/nest/budget/budget.controller.ts index f62bcd68..295c32f2 100644 --- a/server/src/nest/budget/budget.controller.ts +++ b/server/src/nest/budget/budget.controller.ts @@ -229,7 +229,7 @@ export class BudgetController { ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - const member = this.budget.toggleMemberPaid(id, userId, paid); + const member = this.budget.toggleMemberPaid(id, tripId, userId, paid); this.budget.broadcast(tripId, 'budget:member-paid-updated', { itemId: Number(id), userId: Number(userId), paid: paid ? 1 : 0 }, socketId); return { member }; } diff --git a/server/src/nest/budget/budget.service.ts b/server/src/nest/budget/budget.service.ts index 8f4caccc..20bf0434 100644 --- a/server/src/nest/budget/budget.service.ts +++ b/server/src/nest/budget/budget.service.ts @@ -57,8 +57,8 @@ export class BudgetService { return svc.updateMembers(id, tripId, userIds); } - toggleMemberPaid(id: string, userId: string, paid: boolean) { - return svc.toggleMemberPaid(id, userId, paid); + toggleMemberPaid(id: string, tripId: string, userId: string, paid: boolean) { + return svc.toggleMemberPaid(id, tripId, userId, paid); } setPayers(id: string, tripId: string, payers: { user_id: number; amount: number }[]) { diff --git a/server/src/nest/journey/journey-public.controller.ts b/server/src/nest/journey/journey-public.controller.ts index 0bcce1a8..b78e3f0e 100644 --- a/server/src/nest/journey/journey-public.controller.ts +++ b/server/src/nest/journey/journey-public.controller.ts @@ -52,9 +52,11 @@ export class JourneyPublicController { const wantThumb = kind === 'thumbnail' ? 'thumbnail' : 'original'; if (provider === 'local') { - const resolved = path.resolve(path.join(__dirname, '../../../uploads/journey', assetId)); - const uploadsDir = path.resolve(__dirname, '../../../uploads'); - if (!resolved.startsWith(uploadsDir) || !fs.existsSync(resolved)) { + // Local journey assets are flat filenames; use basename() and confine the + // resolved path to the journey upload directory. + const journeyDir = path.resolve(__dirname, '../../../uploads/journey'); + const resolved = path.resolve(path.join(journeyDir, path.basename(assetId))); + if (!resolved.startsWith(journeyDir + path.sep) || !fs.existsSync(resolved)) { throw new HttpException({ error: 'Not found' }, 404); } res.set('Cache-Control', 'public, max-age=86400'); diff --git a/server/src/nest/oidc/oidc.controller.ts b/server/src/nest/oidc/oidc.controller.ts index 23bddb29..8d783603 100644 --- a/server/src/nest/oidc/oidc.controller.ts +++ b/server/src/nest/oidc/oidc.controller.ts @@ -1,6 +1,9 @@ import { Controller, Get, Query, Req, Res } from '@nestjs/common'; import type { Request, Response } from 'express'; import { OidcService } from './oidc.service'; +import { cookieOptions } from '../../services/cookie'; + +const OIDC_STATE_COOKIE = 'trek_oidc_state'; /** * /api/auth/oidc — OIDC SSO login flow (Authorization Code + PKCE). @@ -40,6 +43,11 @@ export class OidcController { const redirectUri = `${appUrl.replace(/\/+$/, '')}/api/auth/oidc/callback`; const inviteToken = req.query.invite as string | undefined; const { state, codeChallenge } = this.oidc.createState(redirectUri, inviteToken); + // Bind the state to THIS browser. The callback requires a matching cookie, + // so an attacker-initiated login (whose callback URL carries a valid state + // from the shared server map) cannot be replayed in a victim's browser to + // log them into the attacker's account (OIDC login CSRF / session fixation). + res.cookie(OIDC_STATE_COOKIE, state, { ...cookieOptions(false, req), maxAge: 10 * 60 * 1000 }); const params = new URLSearchParams({ response_type: 'code', client_id: config.clientId, @@ -61,10 +69,15 @@ export class OidcController { @Query('code') code: string | undefined, @Query('state') state: string | undefined, @Query('error') oidcError: string | undefined, + @Req() req: Request, @Res() res: Response, ): Promise { const f = (p: string) => res.redirect(this.oidc.frontendUrl(p)); + // The state cookie is single-use — clear it regardless of the outcome. + const boundState = (req.cookies as Record | undefined)?.[OIDC_STATE_COOKIE]; + res.clearCookie(OIDC_STATE_COOKIE, cookieOptions(true, req)); + if (!this.oidc.oidcLoginEnabled()) return f('/login?oidc_error=sso_disabled'); if (oidcError) { console.error('[OIDC] Provider error:', oidcError); @@ -72,6 +85,9 @@ export class OidcController { } if (!code || !state) return f('/login?oidc_error=missing_params'); + // Require the callback to come from the browser that started the flow. + if (!boundState || boundState !== state) return f('/login?oidc_error=invalid_state'); + const pending = this.oidc.consumeState(state); if (!pending) return f('/login?oidc_error=invalid_state'); diff --git a/server/src/services/authService.ts b/server/src/services/authService.ts index e7d86b84..43e3a3f9 100644 --- a/server/src/services/authService.ts +++ b/server/src/services/authService.ts @@ -490,8 +490,9 @@ export function loginUser(body: { } if (user.mfa_enabled === 1 || user.mfa_enabled === true) { + const pv = (user as User & { password_version?: number }).password_version ?? 0; const mfa_token = jwt.sign( - { id: Number(user.id), purpose: 'mfa_login' }, + { id: Number(user.id), purpose: 'mfa_login', pv }, JWT_SECRET, { expiresIn: '5m', algorithm: 'HS256' } ); @@ -534,7 +535,7 @@ export function changePassword( userId: number, userEmail: string, body: { current_password?: string; new_password?: string } -): { error?: string; status?: number; success?: boolean } { +): { error?: string; status?: number; success?: boolean; token?: string } { if (isOidcOnlyMode()) { return { error: 'Password authentication is disabled.', status: 403 }; } @@ -549,14 +550,32 @@ export function changePassword( const pwCheck = validatePassword(new_password); if (!pwCheck.ok) return { error: pwCheck.reason, status: 400 }; - const user = db.prepare('SELECT password_hash FROM users WHERE id = ?').get(userId) as { password_hash: string } | undefined; + const user = db.prepare('SELECT password_hash, password_version FROM users WHERE id = ?').get(userId) as { password_hash: string; password_version?: number } | undefined; if (!user || !bcrypt.compareSync(current_password, user.password_hash)) { return { error: 'Current password is incorrect', status: 401 }; } const hash = bcrypt.hashSync(new_password, BCRYPT_COST); - db.prepare('UPDATE users SET password_hash = ?, must_change_password = 0, updated_at = CURRENT_TIMESTAMP WHERE id = ?').run(hash, userId); - return { success: true }; + const newPv = (user.password_version ?? 0) + 1; + + db.transaction(() => { + db.prepare('UPDATE users SET password_hash = ?, must_change_password = 0, password_version = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?').run(hash, newPv, userId); + // A password change rotates the user's sessions: bumping password_version + // invalidates existing JWT cookie sessions, and the separate MCP static + // token and OAuth bearer-token stores are pruned to match (same set the + // password-reset path already revokes). + db.prepare('DELETE FROM mcp_tokens WHERE user_id = ?').run(userId); + try { + db.prepare("UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE user_id = ? AND revoked_at IS NULL").run(userId); + } catch { /* oauth_tokens table may not exist in very old installs */ } + })(); + + try { revokeUserSessions?.(userId); } catch { /* best-effort */ } + + // Re-issue a session bound to the new password_version so the current device + // stays logged in while other existing sessions are rotated out by the pv gate. + const token = generateToken({ id: userId, password_version: newPv }); + return { success: true, token }; } export function deleteAccount(userId: number, userEmail: string, userRole: string): { error?: string; status?: number; success?: boolean } { diff --git a/server/src/services/backupService.ts b/server/src/services/backupService.ts index 9eb54d21..352825f4 100644 --- a/server/src/services/backupService.ts +++ b/server/src/services/backupService.ts @@ -15,7 +15,10 @@ const dataDir = path.join(__dirname, '../../data'); const backupsDir = path.join(dataDir, 'backups'); const uploadsDir = path.join(__dirname, '../../uploads'); -export const MAX_BACKUP_UPLOAD_SIZE = 500 * 1024 * 1024; // 500 MB +export const MAX_BACKUP_UPLOAD_SIZE = 500 * 1024 * 1024; // 500 MB compressed +// Upper bound on the TOTAL decompressed size of a restore archive (the upload +// limit only caps the compressed bytes). Generous enough for any real backup. +export const MAX_BACKUP_DECOMPRESSED_SIZE = 5 * 1024 * 1024 * 1024; // 5 GB // --------------------------------------------------------------------------- // Helpers @@ -187,6 +190,14 @@ export async function restoreFromZip(zipPath: string): Promise { const extractDir = path.join(dataDir, `restore-${Date.now()}`); let reinitFailed: unknown = null; try { + // Check the declared uncompressed size from the central directory and bail + // if it exceeds the cap, before extracting anything. + const directory = await unzipper.Open.file(zipPath); + const claimedSize = directory.files.reduce((sum, f) => sum + (f.uncompressedSize || 0), 0); + if (claimedSize > MAX_BACKUP_DECOMPRESSED_SIZE) { + return { success: false, error: 'Backup exceeds the maximum decompressed size.', status: 400 }; + } + await fs.createReadStream(zipPath) .pipe(unzipper.Extract({ path: extractDir })) .promise(); diff --git a/server/src/services/budgetService.ts b/server/src/services/budgetService.ts index fc98f6ce..8bffe934 100644 --- a/server/src/services/budgetService.ts +++ b/server/src/services/budgetService.ts @@ -280,7 +280,11 @@ export function updateMembers(id: string | number, tripId: string | number, user return { members, item: updated }; } -export function toggleMemberPaid(id: string | number, userId: string | number, paid: boolean) { +export function toggleMemberPaid(id: string | number, tripId: string | number, userId: string | number, paid: boolean) { + // Resolve the item within the caller's trip before updating. + const item = db.prepare('SELECT id FROM budget_items WHERE id = ? AND trip_id = ?').get(id, tripId); + if (!item) return null; + db.prepare('UPDATE budget_item_members SET paid = ? WHERE budget_item_id = ? AND user_id = ?') .run(paid ? 1 : 0, id, userId); diff --git a/server/src/services/journeyService.ts b/server/src/services/journeyService.ts index 3491706a..ad169980 100644 --- a/server/src/services/journeyService.ts +++ b/server/src/services/journeyService.ts @@ -568,8 +568,18 @@ export function updateEntry(entryId: number, userId: number, data: Partial<{ const fields: string[] = []; const values: unknown[] = []; + // Allow-list the columns a client may set: keys come from the request body + // and are interpolated as SQL column names, so restrict them to the known + // entry fields. Keep this in sync with the data type above. + const allowed = new Set([ + 'type', 'title', 'story', 'entry_date', 'entry_time', + 'location_name', 'location_lat', 'location_lng', + 'mood', 'weather', 'tags', 'pros_cons', 'visibility', 'sort_order', + ]); + for (const [key, val] of Object.entries(data)) { if (val === undefined) continue; + if (!allowed.has(key)) continue; if (key === 'tags') { fields.push('tags = ?'); values.push(Array.isArray(val) ? JSON.stringify(val) : val); diff --git a/server/src/services/journeyShareService.ts b/server/src/services/journeyShareService.ts index 5ac03be4..d07e5b78 100644 --- a/server/src/services/journeyShareService.ts +++ b/server/src/services/journeyShareService.ts @@ -84,10 +84,8 @@ export function validateShareTokenForAsset(token: string, assetId: string): { ow JOIN trek_photos tkp ON tkp.id = gp.photo_id WHERE tkp.asset_id = ? AND gp.journey_id = ? `).get(assetId, row.journey_id) as any; - if (!photo) { - const journey = db.prepare('SELECT user_id FROM journeys WHERE id = ?').get(row.journey_id) as any; - return journey ? { ownerId: journey.user_id } : null; - } + // Only resolve assets that actually belong to this shared journey. + if (!photo) return null; return { ownerId: photo.owner_id }; } @@ -137,13 +135,45 @@ export function getPublicJourney(token: string) { photos: photosByEntry[e.id] || [], })); - // Stats + // Stats are derived from the full data so the overview pills stay accurate + // even when a section is hidden. const stats = { entries: entries.length, photos: gallery.length, places: new Set(entries.filter(e => e.location_name).map(e => e.location_name)).size, }; + const shareTimeline = !!row.share_timeline; + const shareGallery = !!row.share_gallery; + const shareMap = !!row.share_map; + + // Honour the share flags server-side so the API only returns the sections the + // owner enabled (the client gates these too, but it must not rely on that). + let publicEntries: Record[] = []; + if (shareTimeline) { + // Include the full entry, but drop GPS unless the map is shared and inline + // photos unless the gallery is shared. + publicEntries = enrichedEntries.map(e => { + const projected: Record = { ...e }; + if (!shareMap) { projected.location_lat = null; projected.location_lng = null; } + if (!shareGallery) projected.photos = []; + return projected; + }); + } else if (shareMap) { + // Map-only share: just enough to plot markers, no story/photos/mood. + publicEntries = enrichedEntries.map(e => ({ + id: e.id, + journey_id: e.journey_id, + type: e.type, + entry_date: e.entry_date, + title: e.title, + location_name: e.location_name, + location_lat: e.location_lat, + location_lng: e.location_lng, + sort_order: e.sort_order, + })); + } + return { journey: { title: journey.title, @@ -151,13 +181,13 @@ export function getPublicJourney(token: string) { cover_image: journey.cover_image, status: journey.status, }, - entries: enrichedEntries, - gallery, + entries: publicEntries, + gallery: shareGallery ? gallery : [], stats, permissions: { - share_timeline: !!row.share_timeline, - share_gallery: !!row.share_gallery, - share_map: !!row.share_map, + share_timeline: shareTimeline, + share_gallery: shareGallery, + share_map: shareMap, }, }; } diff --git a/server/src/services/oidcService.ts b/server/src/services/oidcService.ts index d032c0b7..ebf140fa 100644 --- a/server/src/services/oidcService.ts +++ b/server/src/services/oidcService.ts @@ -28,6 +28,8 @@ export interface OidcTokenResponse { export interface OidcUserInfo { sub: string; email?: string; + // Standard OIDC claim. Some IdPs send it as the string "true"/"false". + email_verified?: boolean | string; name?: string; preferred_username?: string; groups?: string[]; @@ -200,7 +202,11 @@ export function frontendUrl(path: string): string { } export function generateToken(user: { id: number }): string { - return jwt.sign({ id: user.id }, JWT_SECRET, { expiresIn: SESSION_DURATION_SECONDS, algorithm: 'HS256' }); + // Embed the current password_version so an OIDC-issued session is invalidated + // by a password change/reset exactly like a password-login session (the auth + // middleware compares this `pv` against users.password_version). + const pv = (db.prepare('SELECT password_version FROM users WHERE id = ?').get(user.id) as { password_version?: number } | undefined)?.password_version ?? 0; + return jwt.sign({ id: user.id, pv }, JWT_SECRET, { expiresIn: SESSION_DURATION_SECONDS, algorithm: 'HS256' }); } // --------------------------------------------------------------------------- @@ -365,8 +371,14 @@ export function findOrCreateUser( } if (user) { - // Link OIDC identity if not yet linked + // Reaching here without an oidc_sub means we matched an existing local + // account by email. Only auto-link the OIDC identity when the IdP asserts + // the email is verified; an unverified email must not auto-link. if (!user.oidc_sub) { + const emailVerified = userInfo.email_verified === true || userInfo.email_verified === 'true'; + if (!emailVerified) { + return { error: 'email_not_verified' }; + } db.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?').run(sub, config.issuer, user.id); } // Update role based on OIDC claims on every login (if claim mapping is configured) diff --git a/server/src/services/tripService.ts b/server/src/services/tripService.ts index 63ae51b7..413cfdb4 100644 --- a/server/src/services/tripService.ts +++ b/server/src/services/tripService.ts @@ -318,10 +318,12 @@ export function deleteTrip(tripId: string | number, userId: number, userRole: st export function deleteOldCover(coverImage: string | null | undefined) { if (!coverImage) return; - const oldPath = path.join(__dirname, '../../', coverImage.replace(/^\//, '')); - const resolvedPath = path.resolve(oldPath); - const uploadsDir = path.resolve(__dirname, '../../uploads'); - if (resolvedPath.startsWith(uploadsDir) && fs.existsSync(resolvedPath)) { + // cover_image is client-supplied, so treat it as untrusted: covers live in + // uploads/covers as a flat filename — use basename() and confine the unlink + // to that directory. + const coversDir = path.resolve(__dirname, '../../uploads/covers'); + const resolvedPath = path.resolve(path.join(coversDir, path.basename(coverImage))); + if (resolvedPath.startsWith(coversDir + path.sep) && fs.existsSync(resolvedPath)) { fs.unlinkSync(resolvedPath); } } diff --git a/server/tests/integration/oidc.test.ts b/server/tests/integration/oidc.test.ts index 9d6b656a..dd66fffd 100644 --- a/server/tests/integration/oidc.test.ts +++ b/server/tests/integration/oidc.test.ts @@ -165,12 +165,13 @@ describe('GET /api/auth/oidc/callback', () => { sub: 'sub-alice-123', email: 'alice@example.com', name: 'Alice', + email_verified: true, // verified IdP — required to auto-link onto the existing account }); // Create a valid state token const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); - const res = await request(app).get(`/api/auth/oidc/callback?code=authcode123&state=${state}`); + const res = await request(app).get(`/api/auth/oidc/callback?code=authcode123&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); expect(res.status).toBe(302); expect(res.headers.location).toContain('/login?oidc_code='); @@ -188,7 +189,7 @@ describe('GET /api/auth/oidc/callback', () => { const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); - const res = await request(app).get(`/api/auth/oidc/callback?code=code999&state=${state}`); + const res = await request(app).get(`/api/auth/oidc/callback?code=code999&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); expect(res.status).toBe(302); expect(res.headers.location).toContain('/login?oidc_code='); @@ -225,7 +226,7 @@ describe('GET /api/auth/oidc/callback', () => { const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); - const res = await request(app).get(`/api/auth/oidc/callback?code=badcode&state=${state}`); + const res = await request(app).get(`/api/auth/oidc/callback?code=badcode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); expect(res.status).toBe(302); expect(res.headers.location).toContain('oidc_error=token_failed'); @@ -237,7 +238,7 @@ describe('GET /api/auth/oidc/callback', () => { const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); - const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`); + const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); expect(res.status).toBe(302); expect(res.headers.location).toContain('oidc_error=no_id_token'); @@ -250,7 +251,7 @@ describe('GET /api/auth/oidc/callback', () => { const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); - const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`); + const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); expect(res.status).toBe(302); expect(res.headers.location).toContain('oidc_error=id_token_invalid'); @@ -268,7 +269,7 @@ describe('GET /api/auth/oidc/callback', () => { const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); - const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`); + const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); expect(res.status).toBe(302); expect(res.headers.location).toContain('oidc_error=subject_mismatch'); @@ -291,7 +292,7 @@ describe('GET /api/auth/oidc/callback', () => { const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); - const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`); + const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); expect(res.status).toBe(302); expect(res.headers.location).toContain('oidc_error=registration_disabled'); diff --git a/server/tests/unit/middleware/auth.test.ts b/server/tests/unit/middleware/auth.test.ts index d38806e1..78bdda9e 100644 --- a/server/tests/unit/middleware/auth.test.ts +++ b/server/tests/unit/middleware/auth.test.ts @@ -134,6 +134,23 @@ describe('authenticate', () => { expect(next).not.toHaveBeenCalled(); expect(status).toHaveBeenCalledWith(401); }); + + it('AUTH-MW-007: rejects a purpose-scoped mfa_login token even when the user is valid', () => { + // The token issued after the password check but before TOTP is signed with + // the same secret. It must never authenticate a normal request, otherwise + // password alone grants full access and MFA is bypassed. + const mockUser = { id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 0 }; + vi.mocked(db.prepare).mockReturnValue({ get: vi.fn(() => mockUser), all: vi.fn() } as any); + + const mfaToken = jwt.sign({ id: 1, purpose: 'mfa_login', pv: 0 }, 'test-secret', { algorithm: 'HS256' }); + const next = vi.fn() as unknown as NextFunction; + const { res, status } = makeRes(); + + authenticate(makeReq({ headers: { authorization: `Bearer ${mfaToken}` } }), res, next); + + expect(next).not.toHaveBeenCalled(); + expect(status).toHaveBeenCalledWith(401); + }); }); // ── adminOnly ───────────────────────────────────────────────────────────────── diff --git a/server/tests/unit/nest/journey.controller.test.ts b/server/tests/unit/nest/journey.controller.test.ts index 91094eda..f8a6b55f 100644 --- a/server/tests/unit/nest/journey.controller.test.ts +++ b/server/tests/unit/nest/journey.controller.test.ts @@ -1,6 +1,8 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { HttpException } from '@nestjs/common'; import type { Response } from 'express'; +import path from 'node:path'; +import fs from 'node:fs'; import { JourneyController } from '../../../src/nest/journey/journey.controller'; import { JourneyPublicController } from '../../../src/nest/journey/journey-public.controller'; @@ -164,4 +166,27 @@ describe('JourneyPublicController', () => { await new JourneyPublicController(s).legacyPhoto('tok', 'immich', 'a1', '2', 'original', {} as Response); expect(streamImmichAsset).toHaveBeenCalledWith({}, 5, 'a1', 'original', 5); }); + + it('legacy photo proxy: local provider cannot escape uploads/journey via a traversal asset id', async () => { + // Pretend any path exists so we can inspect exactly what would be served. + const existsSpy = vi.spyOn(fs, 'existsSync').mockReturnValue(true); + try { + const sendFile = vi.fn(); + const res = { set: vi.fn(), sendFile } as unknown as Response; + const s = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 5 }) } as Partial); + + // Express decodes %2F in a single path param to '/', so the handler sees this. + await new JourneyPublicController(s).legacyPhoto('tok', 'local', '../../files/secret.pdf', '2', 'original', res); + + expect(sendFile).toHaveBeenCalledTimes(1); + const served = sendFile.mock.calls[0][0] as string; + // basename() collapses the traversal: the served file stays inside + // uploads/journey and never reaches the sibling /uploads/files dir. + expect(path.basename(served)).toBe('secret.pdf'); + expect(served).toMatch(/[\\/]journey[\\/]secret\.pdf$/); + expect(served).not.toMatch(/[\\/]files[\\/]/); + } finally { + existsSpy.mockRestore(); + } + }); }); diff --git a/server/tests/unit/nest/oidc.controller.test.ts b/server/tests/unit/nest/oidc.controller.test.ts index 33a5834c..dcd3b85c 100644 --- a/server/tests/unit/nest/oidc.controller.test.ts +++ b/server/tests/unit/nest/oidc.controller.test.ts @@ -34,11 +34,16 @@ function makeRes() { status: vi.fn((c: number) => { res.statusCode = c; return res; }), json: vi.fn((b: unknown) => { res.body = b; return res; }), redirect: vi.fn((u: string) => { res.redirectedTo = u; }), + cookie: vi.fn(), + clearCookie: vi.fn(), }; return res as unknown as Response & { statusCode: number; redirectedTo: string; body: unknown }; } const req = { query: {}, headers: {} } as Request; +// Callback request carrying the state-binding cookie a real browser would send +// after going through /login. +const reqCb = (state = 's') => ({ query: {}, headers: {}, cookies: { trek_oidc_state: state } } as unknown as Request); beforeEach(() => vi.clearAllMocks()); afterEach(() => { delete process.env.NODE_ENV; }); @@ -71,29 +76,29 @@ describe('OidcController /login', () => { describe('OidcController /callback', () => { it('redirects with sso_disabled when SSO is off', async () => { const res = makeRes(); - await new OidcController(svc({ oidcLoginEnabled: vi.fn().mockReturnValue(false) })).callback('c', 's', undefined, res); + await new OidcController(svc({ oidcLoginEnabled: vi.fn().mockReturnValue(false) })).callback('c', 's', undefined, reqCb('s'), res); expect(res.redirectedTo).toBe('https://app/login?oidc_error=sso_disabled'); }); it('redirects with the provider error', async () => { const res = makeRes(); - await new OidcController(svc()).callback(undefined, undefined, 'access_denied', res); + await new OidcController(svc()).callback(undefined, undefined, 'access_denied', reqCb('s'), res); expect(res.redirectedTo).toBe('https://app/login?oidc_error=access_denied'); }); it('redirects missing_params / invalid_state', async () => { const r1 = makeRes(); - await new OidcController(svc()).callback(undefined, 's', undefined, r1); + await new OidcController(svc()).callback(undefined, 's', undefined, reqCb('s'), r1); expect(r1.redirectedTo).toBe('https://app/login?oidc_error=missing_params'); const r2 = makeRes(); - await new OidcController(svc({ consumeState: vi.fn().mockReturnValue(null) })).callback('c', 's', undefined, r2); + await new OidcController(svc({ consumeState: vi.fn().mockReturnValue(null) })).callback('c', 's', undefined, reqCb('s'), r2); expect(r2.redirectedTo).toBe('https://app/login?oidc_error=invalid_state'); }); it('rejects a missing id_token, then completes with an auth code on success', async () => { vi.spyOn(console, 'error').mockImplementation(() => {}); const noId = makeRes(); - await new OidcController(svc({ exchangeCodeForToken: vi.fn().mockResolvedValue({ _ok: true, access_token: 'at' }) })).callback('c', 's', undefined, noId); + await new OidcController(svc({ exchangeCodeForToken: vi.fn().mockResolvedValue({ _ok: true, access_token: 'at' }) })).callback('c', 's', undefined, reqCb('s'), noId); expect(noId.redirectedTo).toBe('https://app/login?oidc_error=no_id_token'); const ok = makeRes(); @@ -103,10 +108,18 @@ describe('OidcController /callback', () => { getUserInfo: vi.fn().mockResolvedValue({ email: 'a@b.c', sub: 'u1' }), findOrCreateUser: vi.fn().mockReturnValue({ user: { id: 1 } }), })); - await c.callback('c', 's', undefined, ok); + await c.callback('c', 's', undefined, reqCb('s'), ok); expect(ok.redirectedTo).toBe('https://app/login?oidc_code=ac'); }); + it('rejects a callback whose state cookie does not match the query state', async () => { + const res = makeRes(); + // Browser presents a different (or no) state cookie than the callback URL — + // an attacker-initiated flow replayed in the victim's browser. + await new OidcController(svc()).callback('c', 's', undefined, reqCb('attacker-state'), res); + expect(res.redirectedTo).toBe('https://app/login?oidc_error=invalid_state'); + }); + it('rejects a userinfo subject mismatch', async () => { vi.spyOn(console, 'error').mockImplementation(() => {}); const res = makeRes(); @@ -115,7 +128,7 @@ describe('OidcController /callback', () => { verifyIdToken: vi.fn().mockResolvedValue({ ok: true, claims: { sub: 'u1' } }), getUserInfo: vi.fn().mockResolvedValue({ email: 'a@b.c', sub: 'OTHER' }), })); - await c.callback('c', 's', undefined, res); + await c.callback('c', 's', undefined, reqCb('s'), res); expect(res.redirectedTo).toBe('https://app/login?oidc_error=subject_mismatch'); }); }); diff --git a/server/tests/unit/services/authServiceDb.test.ts b/server/tests/unit/services/authServiceDb.test.ts index c4726049..66b2b018 100644 --- a/server/tests/unit/services/authServiceDb.test.ts +++ b/server/tests/unit/services/authServiceDb.test.ts @@ -36,6 +36,7 @@ vi.mock('../../../src/db/database', () => dbMock); vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + SESSION_DURATION_SECONDS: 86400, updateJwtSecret: () => {}, })); vi.mock('../../../src/services/mfaCrypto', () => ({ @@ -88,7 +89,9 @@ import { verifyMfaLogin, createMcpToken, deleteMcpToken, + generateToken, } from '../../../src/services/authService'; +import { verifyJwtAndLoadUser } from '../../../src/middleware/auth'; // --------------------------------------------------------------------------- // Lifecycle @@ -573,6 +576,39 @@ describe('changePassword — OIDC-only mode', () => { }); }); +describe('changePassword — session invalidation', () => { + const pvOf = (id: number) => + (testDb.prepare('SELECT password_version FROM users WHERE id = ?').get(id) as { password_version: number }).password_version; + const mcpCount = (id: number) => + (testDb.prepare('SELECT COUNT(*) c FROM mcp_tokens WHERE user_id = ?').get(id) as { c: number }).c; + + it('AUTH-DB-036b: bumps password_version, prunes MCP tokens, and re-issues a session', () => { + const { user, password } = createUser(testDb); + createMcpToken(user.id, 'cli'); + + expect(pvOf(user.id)).toBe(0); + expect(mcpCount(user.id)).toBe(1); + + const result = changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }); + + expect(result.success).toBe(true); + expect(typeof result.token).toBe('string'); // fresh session for the current device + expect(pvOf(user.id)).toBe(1); // old JWT/cookie sessions now rejected by the pv gate + expect(mcpCount(user.id)).toBe(0); // static MCP tokens revoked + }); + + it('AUTH-DB-036c: a token minted before the change no longer validates afterwards', () => { + const { user, password } = createUser(testDb); + const stolen = generateToken({ id: user.id }); // pv=0 at mint time + + expect(verifyJwtAndLoadUser(stolen)).not.toBeNull(); + + changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }); + + expect(verifyJwtAndLoadUser(stolen)).toBeNull(); // invalidated by the pv bump + }); +}); + // --------------------------------------------------------------------------- // disableMfa — require_mfa policy // --------------------------------------------------------------------------- diff --git a/server/tests/unit/services/backupService.test.ts b/server/tests/unit/services/backupService.test.ts index 120e21c6..470e3c0e 100644 --- a/server/tests/unit/services/backupService.test.ts +++ b/server/tests/unit/services/backupService.test.ts @@ -33,6 +33,9 @@ const archiverMock = vi.hoisted(() => vi.fn()); const unzipperMock = vi.hoisted(() => ({ Extract: vi.fn(), + // Central-directory reader used for the pre-extract zip-bomb size check. + // Default to an empty archive so existing restore tests proceed to Extract. + Open: { file: vi.fn().mockResolvedValue({ files: [] }) }, })); const dbMock = vi.hoisted(() => ({ @@ -532,6 +535,19 @@ describe('BACKUP-038 restoreFromZip', () => { expect(result.error).toMatch(/travel\.db not found/i); expect(result.status).toBe(400); }); + + it('BACKUP-038b — rejects a zip bomb whose declared decompressed size exceeds the cap', async () => { + unzipperMock.Open.file.mockResolvedValueOnce({ + files: [{ uncompressedSize: 6 * 1024 * 1024 * 1024 }], // 6 GB > 5 GB cap + }); + + const result = await restoreFromZip('/data/tmp/bomb.zip'); + + expect(result.success).toBe(false); + expect(result.status).toBe(400); + expect(result.error).toMatch(/decompressed size/i); + expect(unzipperMock.Extract).not.toHaveBeenCalled(); // bailed before extracting + }); }); // --------------------------------------------------------------------------- diff --git a/server/tests/unit/services/budgetServiceDb.test.ts b/server/tests/unit/services/budgetServiceDb.test.ts new file mode 100644 index 00000000..6787920e --- /dev/null +++ b/server/tests/unit/services/budgetServiceDb.test.ts @@ -0,0 +1,83 @@ +/** + * DB-backed unit tests for budgetService trip-scoping (BUDGET-SVC-DB-001+). + * Uses a real in-memory SQLite DB so the SQL WHERE clauses are exercised. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; + +const { testDb, 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'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: () => null, + isOwner: () => false, + }; + return { testDb: db, dbMock: mock }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-secret', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + updateJwtSecret: () => {}, +})); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createTrip } from '../../helpers/factories'; +import { createBudgetItem, updateMembers, toggleMemberPaid } from '../../../src/services/budgetService'; + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); +}); + +afterAll(() => { + testDb.close(); +}); + +function paidFlag(itemId: number, memberId: number): number | undefined { + const row = testDb + .prepare('SELECT paid FROM budget_item_members WHERE budget_item_id = ? AND user_id = ?') + .get(itemId, memberId) as { paid: number } | undefined; + return row?.paid; +} + +describe('toggleMemberPaid trip-scoping', () => { + it('BUDGET-SVC-DB-001: toggles paid for an item that belongs to the given trip', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Trip A' }); + const item = createBudgetItem(trip.id, { name: 'Hotel', total_price: 100 }); + updateMembers(item.id, trip.id, [user.id]); + + const member = toggleMemberPaid(item.id, trip.id, user.id, true); + + expect(member).not.toBeNull(); + expect(paidFlag(item.id, user.id)).toBe(1); + }); + + it('BUDGET-SVC-DB-002: refuses to toggle an item from a different trip (cross-trip IDOR)', () => { + const { user } = createUser(testDb); + const tripA = createTrip(testDb, user.id, { title: 'Trip A' }); + const tripB = createTrip(testDb, user.id, { title: 'Trip B' }); + const itemB = createBudgetItem(tripB.id, { name: 'Foreign expense', total_price: 50 }); + updateMembers(itemB.id, tripB.id, [user.id]); + + // Caller passes a trip they can access (A) but the item lives in trip B. + const member = toggleMemberPaid(itemB.id, tripA.id, user.id, true); + + expect(member).toBeNull(); + expect(paidFlag(itemB.id, user.id)).toBe(0); // unchanged + }); +}); diff --git a/server/tests/unit/services/journeyService.test.ts b/server/tests/unit/services/journeyService.test.ts index 09aa7fe6..fce7dbd2 100644 --- a/server/tests/unit/services/journeyService.test.ts +++ b/server/tests/unit/services/journeyService.test.ts @@ -596,6 +596,32 @@ describe('updateEntry', () => { expect(result).toBeNull(); }); + + it('JOURNEY-SVC-034b: ignores injection column keys and mass-assignment attempts', () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const entry = createJourneyEntry(testDb, journey.id, user.id, { + title: 'Safe', + story: 'original', + entry_date: '2026-03-01', + }); + + // The keys come straight from the request body. A crafted key was previously + // interpolated as a raw SQL column name (`${key} = ?`), enabling subquery + // injection (full DB read) and mass-assignment of protected columns. + const malicious: Record = { + title: 'Updated', + [`story = (SELECT password_hash FROM users WHERE id = ${user.id}), updated_at`]: 'x', + author_id: 999999, + }; + + const updated = updateEntry(entry.id, user.id, malicious as Parameters[2]); + + expect(updated).not.toBeNull(); + expect(updated!.title).toBe('Updated'); // legit field still applied + expect(updated!.story).toBe('original'); // injection key dropped — no hash leaked into story + expect(updated!.author_id).toBe(user.id); // mass-assignment blocked + }); }); describe('deleteEntry', () => { diff --git a/server/tests/unit/services/journeyShareService.test.ts b/server/tests/unit/services/journeyShareService.test.ts index 3df9c2ff..3cced54c 100644 --- a/server/tests/unit/services/journeyShareService.test.ts +++ b/server/tests/unit/services/journeyShareService.test.ts @@ -300,15 +300,17 @@ describe('validateShareTokenForAsset', () => { expect(result).toBeNull(); }); - it('JOURNEY-SHARE-015: falls back to journey owner when asset not found in photos', () => { + it('JOURNEY-SHARE-015: denies (returns null) when the asset is not part of the shared journey', () => { const { user } = createUser(testDb); const journey = createJourney(testDb, user.id); const { token } = createOrUpdateJourneyShareLink(journey.id, user.id, {}); + // A valid share token must NOT resolve arbitrary asset IDs to the owner — + // otherwise it could proxy any asset out of the owner's Immich/Synology + // library (IDOR). Only assets actually in the journey may resolve. const result = validateShareTokenForAsset(token, 'nonexistent-asset'); - expect(result).not.toBeNull(); - expect(result!.ownerId).toBe(user.id); + expect(result).toBeNull(); }); }); @@ -414,4 +416,76 @@ describe('getPublicJourney', () => { expect(result!.stats.photos).toBe(0); expect(result!.stats.places).toBe(0); }); + + it('JOURNEY-SHARE-021: withholds timeline, gallery and GPS when all flags are off', () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id, { title: 'Secret' }); + const entry = createJourneyEntry(testDb, journey.id, user.id, { + type: 'entry', title: 'Day 1', story: 'private notes', entry_date: '2026-05-01', location_name: 'Paris', + }); + testDb.prepare('UPDATE journey_entries SET location_lat = ?, location_lng = ? WHERE id = ?').run(48.8566, 2.3522, entry.id); + insertJourneyPhoto(entry.id); + const { token } = createOrUpdateJourneyShareLink(journey.id, user.id, { + share_timeline: false, share_gallery: false, share_map: false, + }); + + const result = getPublicJourney(token)!; + expect(result.entries).toEqual([]); // no timeline / story / GPS leaked + expect(result.gallery).toEqual([]); // no gallery leaked + expect(result.stats.entries).toBe(1); // counts stay accurate + }); + + it('JOURNEY-SHARE-022: shares the timeline but strips GPS when the map flag is off', () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const entry = createJourneyEntry(testDb, journey.id, user.id, { + type: 'entry', title: 'Day 1', story: 'notes', entry_date: '2026-05-01', location_name: 'Paris', + }); + testDb.prepare('UPDATE journey_entries SET location_lat = ?, location_lng = ? WHERE id = ?').run(48.8566, 2.3522, entry.id); + const { token } = createOrUpdateJourneyShareLink(journey.id, user.id, { + share_timeline: true, share_gallery: true, share_map: false, + }); + + const result = getPublicJourney(token)!; + expect(result.entries).toHaveLength(1); + const e = result.entries[0] as Record; + expect(e.story).toBe('notes'); // narrative present + expect(e.location_lat).toBeNull(); // GPS withheld + expect(e.location_lng).toBeNull(); + }); + + it('JOURNEY-SHARE-023: map-only share exposes coordinates but not the story', () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const entry = createJourneyEntry(testDb, journey.id, user.id, { + type: 'entry', title: 'Day 1', story: 'private notes', entry_date: '2026-05-01', location_name: 'Paris', + }); + testDb.prepare('UPDATE journey_entries SET location_lat = ?, location_lng = ? WHERE id = ?').run(48.8566, 2.3522, entry.id); + const { token } = createOrUpdateJourneyShareLink(journey.id, user.id, { + share_timeline: false, share_gallery: false, share_map: true, + }); + + const result = getPublicJourney(token)!; + expect(result.entries).toHaveLength(1); + const e = result.entries[0] as Record; + expect(e.location_lat).toBe(48.8566); // coords for the map + expect(e.story).toBeUndefined(); // narrative withheld + }); + + it('JOURNEY-SHARE-024: strips inline entry photos (and their asset metadata) when the gallery is off', () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const entry = createJourneyEntry(testDb, journey.id, user.id, { + type: 'entry', title: 'Day 1', story: 'notes', entry_date: '2026-05-01', + }); + insertJourneyPhoto(entry.id, { ownerId: user.id }); + const { token } = createOrUpdateJourneyShareLink(journey.id, user.id, { + share_timeline: true, share_gallery: false, share_map: true, + }); + + const result = getPublicJourney(token)!; + expect(result.gallery).toEqual([]); // gallery array withheld + expect(result.entries).toHaveLength(1); + expect((result.entries[0] as Record).photos).toEqual([]); // inline photos withheld too + }); }); diff --git a/server/tests/unit/services/oidcService.test.ts b/server/tests/unit/services/oidcService.test.ts index 2b3c16f0..6a86b2cc 100644 --- a/server/tests/unit/services/oidcService.test.ts +++ b/server/tests/unit/services/oidcService.test.ts @@ -313,7 +313,7 @@ describe('findOrCreateUser', () => { const { user } = createUser(testDb, { email: 'bob@example.com' }); const result = findOrCreateUser( - { sub: 'sub-bob-new', email: 'bob@example.com', name: 'Bob' }, + { sub: 'sub-bob-new', email: 'bob@example.com', name: 'Bob', email_verified: true }, MOCK_CONFIG ); expect('user' in result).toBe(true); @@ -352,13 +352,13 @@ describe('findOrCreateUser', () => { expect((result as { error: string }).error).toBe('registration_disabled'); }); - it('OIDC-SVC-025: links oidc_sub when existing user has none', () => { + it('OIDC-SVC-025: links oidc_sub when existing user has none (verified email)', () => { const { user } = createUser(testDb, { email: 'charlie@example.com' }); // Ensure no oidc_sub set testDb.prepare('UPDATE users SET oidc_sub = NULL, oidc_issuer = NULL WHERE id = ?').run(user.id); findOrCreateUser( - { sub: 'sub-charlie-linked', email: 'charlie@example.com', name: 'Charlie' }, + { sub: 'sub-charlie-linked', email: 'charlie@example.com', name: 'Charlie', email_verified: true }, MOCK_CONFIG ); @@ -366,6 +366,23 @@ describe('findOrCreateUser', () => { expect(updated.oidc_sub).toBe('sub-charlie-linked'); }); + it('OIDC-SVC-025b: refuses to link an unverified email to an existing local account', () => { + const { user } = createUser(testDb, { email: 'dora@example.com' }); + testDb.prepare('UPDATE users SET oidc_sub = NULL, oidc_issuer = NULL WHERE id = ?').run(user.id); + + // No email_verified claim — an IdP that lets users set arbitrary emails must + // not be able to take over a pre-existing password account. + const result = findOrCreateUser( + { sub: 'sub-dora-attacker', email: 'dora@example.com', name: 'Dora' }, + MOCK_CONFIG + ); + + expect('error' in result).toBe(true); + expect((result as { error: string }).error).toBe('email_not_verified'); + const updated = testDb.prepare('SELECT oidc_sub FROM users WHERE id = ?').get(user.id) as any; + expect(updated.oidc_sub).toBeNull(); // account not linked / not hijacked + }); + it('OIDC-SVC-026: existing user role is updated when OIDC claim mapping changes it', () => { const { user } = createUser(testDb, { email: 'diana@example.com', role: 'user' }); // Link oidc_sub manually so the user is found by sub lookup diff --git a/server/tests/unit/services/tripService.test.ts b/server/tests/unit/services/tripService.test.ts index a4411b4d..a6c876c5 100644 --- a/server/tests/unit/services/tripService.test.ts +++ b/server/tests/unit/services/tripService.test.ts @@ -34,7 +34,8 @@ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, createReservation, createPlace, createDay, createDayAssignment, createDayNote } from '../../helpers/factories'; -import { exportICS, generateDays } from '../../../src/services/tripService'; +import { exportICS, generateDays, deleteOldCover } from '../../../src/services/tripService'; +import fs from 'fs'; beforeAll(() => { createTables(testDb); @@ -397,3 +398,41 @@ describe('exportICS', () => { expect(ics).toContain('DTEND:20250602T160000'); }); }); + +// ── deleteOldCover — path containment ────────────────────────────────────────── + +describe('deleteOldCover', () => { + it('TRIP-SVC-COVER-001: never unlinks outside uploads/covers for a crafted cover_image', () => { + const existsSpy = vi.spyOn(fs, 'existsSync').mockReturnValue(true); + const unlinkSpy = vi.spyOn(fs, 'unlinkSync').mockImplementation(() => {}); + try { + // Attacker-controlled values aimed at auth-gated sibling upload dirs. + deleteOldCover('/uploads/files/victim.pdf'); + deleteOldCover('/uploads/covers/../files/secret.pdf'); + deleteOldCover('/uploads/avatars/someone.png'); + + for (const call of unlinkSpy.mock.calls) { + const target = String(call[0]); + expect(target).toMatch(/[\\/]uploads[\\/]covers[\\/]/); // stays in covers + expect(target).not.toMatch(/[\\/]files[\\/]/); + expect(target).not.toMatch(/[\\/]avatars[\\/]/); + } + } finally { + existsSpy.mockRestore(); + unlinkSpy.mockRestore(); + } + }); + + it('TRIP-SVC-COVER-002: deletes a legitimate cover file', () => { + const existsSpy = vi.spyOn(fs, 'existsSync').mockReturnValue(true); + const unlinkSpy = vi.spyOn(fs, 'unlinkSync').mockImplementation(() => {}); + try { + deleteOldCover('/uploads/covers/abc123.jpg'); + expect(unlinkSpy).toHaveBeenCalledTimes(1); + expect(String(unlinkSpy.mock.calls[0][0])).toMatch(/[\\/]covers[\\/]abc123\.jpg$/); + } finally { + existsSpy.mockRestore(); + unlinkSpy.mockRestore(); + } + }); +});