From 1239be4259165c7e1b83585c04957b30bf868cdf Mon Sep 17 00:00:00 2001 From: jubnl Date: Tue, 11 Aug 2026 23:11:34 +0200 Subject: [PATCH] refactor(server): fold the _shared MCP helpers into an injectable guards provider; delete the last bridge --- server/src/mcp/tools/_shared.ts | 37 +----- .../nest/accommodations/accommodations.mcp.ts | 22 ++-- .../accommodations/accommodations.module.ts | 3 +- .../src/nest/assignments/assignments.mcp.ts | 28 ++--- .../nest/assignments/assignments.module.ts | 3 +- server/src/nest/budget/budget.mcp.ts | 42 +++---- server/src/nest/budget/budget.module.ts | 3 +- server/src/nest/collab/collab.mcp.ts | 44 +++---- server/src/nest/collab/collab.module.ts | 3 +- server/src/nest/day-notes/day-notes.mcp.ts | 21 ++-- server/src/nest/day-notes/day-notes.module.ts | 3 +- server/src/nest/days/days.mcp.ts | 16 +-- server/src/nest/days/days.module.ts | 3 +- .../src/nest/mcp-shared/mcp-shared.module.ts | 16 +++ .../mcp-shared/mcp-tool-guards.service.ts | 55 +++++++++ server/src/nest/packing/packing.mcp.ts | 58 +++++----- server/src/nest/packing/packing.module.ts | 3 +- .../nest/permissions/permissions.bridge.ts | 29 ----- server/src/nest/places/places.mcp.ts | 36 +++--- server/src/nest/places/places.module.ts | 3 +- .../src/nest/reservations/reservations.mcp.ts | 46 ++++---- .../nest/reservations/reservations.module.ts | 3 +- server/src/nest/share/share.mcp.ts | 15 ++- server/src/nest/share/share.module.ts | 3 +- server/src/nest/todo/todo.mcp.ts | 26 +++-- server/src/nest/todo/todo.module.ts | 3 +- server/src/nest/transit/transit.mcp.ts | 8 +- server/src/nest/transit/transit.module.ts | 3 +- server/src/nest/trips/trips.mcp.ts | 12 +- server/src/nest/trips/trips.module.ts | 3 +- server/tests/e2e/transit.e2e.test.ts | 3 - server/tests/helpers/mcp-test-controllers.ts | 28 ++--- server/tests/unit/mcp/tools-prompts.test.ts | 6 +- .../unit/nest/mcp-shared.service.test.ts | 108 ++++++++++++++++++ .../unit/nest/permissions.service.test.ts | 21 ++-- 35 files changed, 443 insertions(+), 273 deletions(-) create mode 100644 server/src/nest/mcp-shared/mcp-shared.module.ts create mode 100644 server/src/nest/mcp-shared/mcp-tool-guards.service.ts delete mode 100644 server/src/nest/permissions/permissions.bridge.ts create mode 100644 server/tests/unit/nest/mcp-shared.service.test.ts diff --git a/server/src/mcp/tools/_shared.ts b/server/src/mcp/tools/_shared.ts index 2f21ca276..b6bc618bf 100644 --- a/server/src/mcp/tools/_shared.ts +++ b/server/src/mcp/tools/_shared.ts @@ -1,11 +1,10 @@ import { errorResult } from '@trek/nest-mcp'; -import { db } from '../../db/database'; -import { checkPermission } from '../../nest/permissions/permissions.bridge'; -import { broadcast } from '../../websocket'; // Generic MCP result helpers and annotation presets live in @trek/nest-mcp -// (single source for the legacy registrars below and the decorator-based -// domains). Re-exported so registrar files keep importing from here. +// (single source for the decorator-based domains). Re-exported so the +// @McpController files keep importing from here. Everything below is PURE — +// the impure guards (hasTripPermission/isAdminUser/safeBroadcast) moved onto +// the injectable McpToolGuardsService (src/nest/mcp-shared/). export { demoDenied, errorResult, @@ -18,14 +17,6 @@ export { TOOL_ANNOTATIONS_WRITE, } from '@trek/nest-mcp'; -export function safeBroadcast(tripId: number, event: string, payload: Record): void { - try { - broadcast(tripId, event, { ...payload, _source: 'mcp' }); - } catch (err) { - console.error(`[MCP] broadcast failed for ${event}:`, err?.message ?? err); - } -} - export const MAX_MCP_TRIP_DAYS = 90; export function noAccess() { @@ -36,26 +27,6 @@ export function permissionDenied() { return errorResult('You do not have permission to perform this action on this trip.'); } -/** - * 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); -} - -/** True when the user has the global admin role (mirrors REST `user.role === 'admin'` gates). */ -export function isAdminUser(userId: number): boolean { - const userRow = db.prepare('SELECT role FROM users WHERE id = ?').get(userId) as { role?: string } | undefined; - return userRow?.role === 'admin'; -} - /** Error response for admin-only tools, reproducing the REST `{ error: 'Admin access required' }` string. */ export function adminRequired() { return errorResult('Admin access required'); diff --git a/server/src/nest/accommodations/accommodations.mcp.ts b/server/src/nest/accommodations/accommodations.mcp.ts index 0a8cb705c..1c32f6737 100644 --- a/server/src/nest/accommodations/accommodations.mcp.ts +++ b/server/src/nest/accommodations/accommodations.mcp.ts @@ -3,10 +3,11 @@ import { TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; import { PlacesService } from '../places/places.service'; -import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { DatabaseService } from '../database/database.service'; import { AccommodationsService } from './accommodations.service'; @@ -32,6 +33,7 @@ export class AccommodationsMcp { private readonly db: DatabaseService, private readonly places: PlacesService, private readonly auth: AuthService, + private readonly guards: McpToolGuardsService, ) {} @Tool({ @@ -60,11 +62,11 @@ export class AccommodationsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.accommodations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const errors = this.accommodations.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 = this.accommodations.createAccommodation(tripId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes }); - safeBroadcast(tripId, 'accommodation:created', { accommodation }); + this.guards.safeBroadcast(tripId, 'accommodation:created', { accommodation }); return ok({ accommodation }); } @@ -110,7 +112,7 @@ export class AccommodationsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.accommodations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const dayErrors = this.accommodations.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 { @@ -119,8 +121,8 @@ export class AccommodationsMcp { const accommodation = this.accommodations.createAccommodation(tripId, { place_id: place.id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes: accommodation_notes }); return { place, accommodation }; }); - safeBroadcast(tripId, 'place:created', { place: result.place }); - safeBroadcast(tripId, 'accommodation:created', { accommodation: result.accommodation }); + this.guards.safeBroadcast(tripId, 'place:created', { place: result.place }); + this.guards.safeBroadcast(tripId, 'accommodation:created', { accommodation: result.accommodation }); return ok(result); } catch { return { content: [{ type: 'text' as const, text: 'Failed to create place and accommodation.' }], isError: true }; @@ -154,11 +156,11 @@ export class AccommodationsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.accommodations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const existing = this.accommodations.getAccommodation(accommodationId, tripId); if (!existing) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true }; const accommodation = this.accommodations.updateAccommodation(accommodationId, existing, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes }); - safeBroadcast(tripId, 'accommodation:updated', { accommodation }); + this.guards.safeBroadcast(tripId, 'accommodation:updated', { accommodation }); return ok({ accommodation }); } @@ -175,10 +177,10 @@ export class AccommodationsMcp { async deleteAccommodation({ tripId, accommodationId }: { tripId: number; accommodationId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.accommodations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.accommodations.getAccommodation(accommodationId, tripId)) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true }; const { linkedReservationId } = this.accommodations.deleteAccommodation(accommodationId); - safeBroadcast(tripId, 'accommodation:deleted', { id: accommodationId, linkedReservationId }); + this.guards.safeBroadcast(tripId, 'accommodation:deleted', { id: accommodationId, linkedReservationId }); return ok({ success: true, linkedReservationId }); } diff --git a/server/src/nest/accommodations/accommodations.module.ts b/server/src/nest/accommodations/accommodations.module.ts index a6b9181a1..96e41b831 100644 --- a/server/src/nest/accommodations/accommodations.module.ts +++ b/server/src/nest/accommodations/accommodations.module.ts @@ -9,6 +9,7 @@ import { DatabaseModule } from '../database/database.module'; import { PermissionsModule } from '../permissions/permissions.module'; import { RealtimeModule } from '../realtime/realtime.module'; import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** * Accommodations. One fachlichkeit, one module: the routes used to sit in @@ -21,7 +22,7 @@ import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; * the same reason DaysMcp could drop it. */ @Module({ - imports: [PermissionsModule, RealtimeModule, PluginGuardsModule, DatabaseModule, PlacesModule, AuthModule], + imports: [McpSharedModule, PermissionsModule, RealtimeModule, PluginGuardsModule, DatabaseModule, PlacesModule, AuthModule], controllers: [AccommodationsController], providers: [AccommodationsService, AccommodationsRpc, AccommodationsMcp], exports: [AccommodationsService], diff --git a/server/src/nest/assignments/assignments.mcp.ts b/server/src/nest/assignments/assignments.mcp.ts index 4329b955b..68b843a96 100644 --- a/server/src/nest/assignments/assignments.mcp.ts +++ b/server/src/nest/assignments/assignments.mcp.ts @@ -4,9 +4,10 @@ import { TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, errorResult, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { AssignmentsService } from './assignments.service'; import { DaysService } from '../days/days.service'; @@ -25,6 +26,7 @@ export class AssignmentsMcp { private readonly assignments: AssignmentsService, private readonly days: DaysService, private readonly auth: AuthService, + private readonly guards: McpToolGuardsService, ) {} @Tool({ @@ -45,11 +47,11 @@ export class AssignmentsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.assignments.dayExists(dayId, tripId)) return errorResult('Day not found.'); if (!this.assignments.placeExists(placeId, tripId)) return errorResult('Place not found.'); const assignment = this.assignments.createAssignment(dayId, placeId, notes || null); - safeBroadcast(tripId, 'assignment:created', { assignment }); + this.guards.safeBroadcast(tripId, 'assignment:created', { assignment }); this.assignments.reconcile(tripId); return ok({ assignment }); } @@ -71,11 +73,11 @@ export class AssignmentsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.assignments.assignmentExistsInDay(assignmentId, dayId, tripId)) return errorResult('Assignment not found.'); this.assignments.deleteAssignment(assignmentId); - safeBroadcast(tripId, 'assignment:deleted', { assignmentId, dayId }); + this.guards.safeBroadcast(tripId, 'assignment:deleted', { assignmentId, dayId }); this.assignments.reconcile(tripId); return ok({ success: true }); } @@ -100,7 +102,7 @@ export class AssignmentsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const existing = this.assignments.getAssignmentForTrip(assignmentId, tripId); if (!existing) return errorResult('Assignment not found.'); const assignment = this.assignments.updateTime( @@ -108,7 +110,7 @@ export class AssignmentsMcp { place_time !== undefined ? place_time : existing.assignment_time, end_time !== undefined ? end_time : existing.assignment_end_time ); - safeBroadcast(tripId, 'assignment:updated', { assignment }); + this.guards.safeBroadcast(tripId, 'assignment:updated', { assignment }); this.assignments.reconcile(tripId); return ok({ assignment }); } @@ -134,14 +136,14 @@ export class AssignmentsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); if (!this.days.getDay(newDayId, tripId)) return errorResult('Day not found.'); const result = this.assignments.moveAssignment(assignmentId, newDayId, orderIndex ?? 0); // REST parity shape ({ assignment, oldDayId, newDayId }) — the client keys its // per-day assignment map on newDayId, so omitting it filed the moved assignment // under "undefined" on collaborator screens. - safeBroadcast(tripId, 'assignment:moved', { assignment: result.assignment, oldDayId: result.oldDayId, newDayId }); + this.guards.safeBroadcast(tripId, 'assignment:moved', { assignment: result.assignment, oldDayId: result.oldDayId, newDayId }); this.assignments.reconcile(tripId); return ok({ assignment: result.assignment }); } @@ -183,10 +185,10 @@ export class AssignmentsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); const participants = this.assignments.setParticipants(assignmentId, userIds); - safeBroadcast(tripId, 'assignment:participants', { assignmentId, participants }); + this.guards.safeBroadcast(tripId, 'assignment:participants', { assignmentId, participants }); return ok({ participants }); } @@ -207,12 +209,12 @@ export class AssignmentsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.days.getDay(dayId, tripId)) return errorResult('Day not found.'); this.assignments.reorderAssignments(dayId, assignmentIds); // REST parity shape ({ dayId, orderedIds }) — the client only reads orderedIds, // so broadcasting the tool-input key name emptied the day for collaborators. - safeBroadcast(tripId, 'assignment:reordered', { dayId, orderedIds: assignmentIds }); + this.guards.safeBroadcast(tripId, 'assignment:reordered', { dayId, orderedIds: assignmentIds }); return ok({ success: true, dayId, order: assignmentIds }); } } diff --git a/server/src/nest/assignments/assignments.module.ts b/server/src/nest/assignments/assignments.module.ts index 3106fd735..46d8ef979 100644 --- a/server/src/nest/assignments/assignments.module.ts +++ b/server/src/nest/assignments/assignments.module.ts @@ -8,6 +8,7 @@ import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; import { RealtimeModule } from '../realtime/realtime.module'; import { AssignmentsMcp } from './assignments.mcp'; import { AuthModule } from '../auth/auth.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** * Assignments domain (S7 — Phase 2 trip sub-domain). The day-assignments mount @@ -20,7 +21,7 @@ import { AuthModule } from '../auth/auth.module'; @Module({ // DaysModule: AssignmentsMcp injects DaysService for the target-day checks. // PermissionsModule: the controllers' TripAccessGuard injects PermissionsService. - imports: [AssignmentsDomainModule, DaysModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule], + imports: [McpSharedModule, AssignmentsDomainModule, DaysModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule], controllers: [DayAssignmentsController, AssignmentOpsController], providers: [AssignmentsMcp, ItineraryRpc], exports: [AssignmentsDomainModule], diff --git a/server/src/nest/budget/budget.mcp.ts b/server/src/nest/budget/budget.mcp.ts index 8177c0f72..49f1602dd 100644 --- a/server/src/nest/budget/budget.mcp.ts +++ b/server/src/nest/budget/budget.mcp.ts @@ -4,11 +4,12 @@ import { TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, errorResult, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { RuntimeEnvService } from '../app-config/runtime-env.service'; import { isDemoUserId } from '../common/demo-write'; import { ADDON_IDS } from '../../addons'; -import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { TripMembershipService } from '../trip-membership/trip-membership.service'; import { DatabaseService } from '../database/database.service'; import { BudgetService } from './budget.service'; @@ -59,6 +60,7 @@ export class BudgetMcp { private readonly env: RuntimeEnvService, private readonly membership: TripMembershipService, readonly addons: AddonsService, + private readonly guards: McpToolGuardsService, ) {} /** The AuthService.isDemoUser check without the auth graph (demo-write.ts). */ @@ -110,14 +112,14 @@ export class BudgetMcp { ) { if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); const members = this.resolveMemberIds(tripId, member_ids); const itemData = { category, name, total_price, currency, member_ids: members, payers, expense_date, note }; // Freeze the live FX rate at entry time so a settled position isn't re-opened // when live rates drift (#1445) — same as the REST create path. await this.budget.freezeForeignRate(tripId, itemData); const item = this.budget.createBudgetItem(tripId, itemData); - safeBroadcast(tripId, 'budget:created', { item }); + this.guards.safeBroadcast(tripId, 'budget:created', { item }); return ok({ item }); } @@ -135,10 +137,10 @@ export class BudgetMcp { async deleteBudgetItem({ tripId, itemId }: { tripId: number; itemId: number }, ctx: McpContext) { if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); const deleted = this.budget.deleteBudgetItem(itemId, tripId); if (!deleted) return errorResult('Budget item not found.'); - safeBroadcast(tripId, 'budget:deleted', { itemId }); + this.guards.safeBroadcast(tripId, 'budget:deleted', { itemId }); return ok({ success: true }); } @@ -172,12 +174,12 @@ export class BudgetMcp { ) { if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); // Freeze-then-write composite (no-op while the schema has no currency input, // but keeps REST and MCP on one code path for the #1445 freeze). const item = await this.budget.update(itemId, tripId, { name, category, total_price, member_ids, payers, persons, days, note }); if (!item) return errorResult('Budget item not found.'); - safeBroadcast(tripId, 'budget:updated', { item }); + this.guards.safeBroadcast(tripId, 'budget:updated', { item }); return ok({ item }); } @@ -206,7 +208,7 @@ export class BudgetMcp { ) { if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); // Omitted userIds → default to the whole trip, matching create_budget_item. const members = (userIds && userIds.length > 0) ? userIds : this.resolveMemberIds(tripId, undefined); try { @@ -214,8 +216,8 @@ export class BudgetMcp { const created = this.budget.createBudgetItem(tripId, { category, name, total_price, note, member_ids: members }); return this.budget.getBudgetItem(created.id, tripId)!; }); - safeBroadcast(tripId, 'budget:created', { item }); - if (members && members.length > 0) safeBroadcast(tripId, 'budget:members-updated', { itemId: item.id, members: item.members, persons: item.persons }); + this.guards.safeBroadcast(tripId, 'budget:created', { item }); + if (members && members.length > 0) this.guards.safeBroadcast(tripId, 'budget:members-updated', { itemId: item.id, members: item.members, persons: item.persons }); return ok({ item }); } catch { return errorResult('Failed to create budget item.'); @@ -237,11 +239,11 @@ export class BudgetMcp { async setBudgetItemMembers({ tripId, itemId, userIds }: { tripId: number; itemId: number; userIds: number[] }, ctx: McpContext) { if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); const result = this.budget.updateMembers(itemId, tripId, userIds); if (!result) return errorResult('Budget item not found.'); const item = this.budget.getBudgetItem(itemId, tripId); - safeBroadcast(tripId, 'budget:members-updated', { itemId, members: result.members, persons: result.item.persons }); + this.guards.safeBroadcast(tripId, 'budget:members-updated', { itemId, members: result.members, persons: result.item.persons }); return ok({ item }); } @@ -261,9 +263,9 @@ export class BudgetMcp { async toggleBudgetMemberPaid({ tripId, itemId, memberId, paid }: { tripId: number; itemId: number; memberId: number; paid: boolean }, ctx: McpContext) { if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); const member = this.budget.toggleMemberPaid(itemId, tripId, memberId, paid); - safeBroadcast(tripId, 'budget:member-paid-updated', { itemId, userId: memberId, paid: paid ? 1 : 0 }); + this.guards.safeBroadcast(tripId, 'budget:member-paid-updated', { itemId, userId: memberId, paid: paid ? 1 : 0 }); return ok({ member }); } @@ -324,11 +326,11 @@ export class BudgetMcp { ) { if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); // Freeze-then-write composite, same as the REST path (no-op while the // schema has no currency input — see the #1445 note in the class doc). const settlement = await this.budget.createSettlement(tripId, { from_user_id, to_user_id, amount }, ctx.userId); - safeBroadcast(tripId, 'budget:settlement-created', { settlement }); + this.guards.safeBroadcast(tripId, 'budget:settlement-created', { settlement }); return ok({ settlement }); } @@ -352,11 +354,11 @@ export class BudgetMcp { ) { if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); // Freeze-then-write composite, same as the REST path. const settlement = await this.budget.updateSettlement(settlementId, tripId, { from_user_id, to_user_id, amount }); if (!settlement) return errorResult('Settlement not found.'); - safeBroadcast(tripId, 'budget:settlement-updated', { settlement }); + this.guards.safeBroadcast(tripId, 'budget:settlement-updated', { settlement }); return ok({ settlement }); } @@ -374,10 +376,10 @@ export class BudgetMcp { async deleteSettlement({ tripId, settlementId }: { tripId: number; settlementId: number }, ctx: McpContext) { if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); const deleted = this.budget.deleteSettlement(settlementId, tripId); if (!deleted) return errorResult('Settlement not found.'); - safeBroadcast(tripId, 'budget:settlement-deleted', { settlementId }); + this.guards.safeBroadcast(tripId, 'budget:settlement-deleted', { settlementId }); return ok({ success: true }); } diff --git a/server/src/nest/budget/budget.module.ts b/server/src/nest/budget/budget.module.ts index 7dfa974ad..4aef17217 100644 --- a/server/src/nest/budget/budget.module.ts +++ b/server/src/nest/budget/budget.module.ts @@ -11,6 +11,7 @@ import { PermissionsModule } from '../permissions/permissions.module'; import { AppConfigModule } from '../app-config/app-config.module'; import { TripMembershipModule } from '../trip-membership/trip-membership.module'; import { AddonsModule } from '../addons/addons.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** Budget domain (S4 — Phase 2 trip sub-domain). Registered in AppModule. * BudgetMcp carries the decorator-registered MCP tools + resources. @@ -20,7 +21,7 @@ import { AddonsModule } from '../addons/addons.module'; * AppConfigModule is @Global in the app graph; the explicit import keeps the * partial e2e TestingModules resolving RuntimeEnvService. */ @Module({ - imports: [PermissionsModule, AppConfigModule, RealtimeModule, PluginGuardsModule, AddonsModule, TripMembershipModule], + imports: [McpSharedModule, PermissionsModule, AppConfigModule, RealtimeModule, PluginGuardsModule, AddonsModule, TripMembershipModule], controllers: [BudgetController], providers: [BudgetService, ExchangeRatesService, BudgetMcp, ExchangeRatesRpc, CostsRpc], // For in-container consumers (CostsRpc, TripsService, diff --git a/server/src/nest/collab/collab.mcp.ts b/server/src/nest/collab/collab.mcp.ts index 59dce4852..7a4bec645 100644 --- a/server/src/nest/collab/collab.mcp.ts +++ b/server/src/nest/collab/collab.mcp.ts @@ -4,10 +4,11 @@ import { TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, errorResult, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; import { ADDON_IDS } from '../../addons'; -import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { CollabService } from './collab.service'; import { collabFeatureGate } from '../addons/addon-gate'; import { AddonsService } from '../addons/addons.service'; @@ -64,6 +65,7 @@ export class CollabMcp { private readonly collab: CollabService, private readonly auth: AuthService, readonly addons: AddonsService, + private readonly guards: McpToolGuardsService, ) {} // --- COLLAB NOTES --- @@ -91,9 +93,9 @@ export class CollabMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); const note = this.collab.createNote(tripId, ctx.userId, { title, content, category, color, pinned }); - safeBroadcast(tripId, 'collab:note:created', { note }); + this.guards.safeBroadcast(tripId, 'collab:note:created', { note }); return ok({ note }); } @@ -121,10 +123,10 @@ export class CollabMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); const note = this.collab.updateNote(tripId, noteId, { title, content, category, color, pinned }); if (!note) return errorResult('Note not found.'); - safeBroadcast(tripId, 'collab:note:updated', { note }); + this.guards.safeBroadcast(tripId, 'collab:note:updated', { note }); return ok({ note }); } @@ -142,10 +144,10 @@ export class CollabMcp { async deleteCollabNote({ tripId, noteId }: { tripId: number; noteId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); const deleted = this.collab.deleteNote(tripId, noteId); if (!deleted) return errorResult('Note not found.'); - safeBroadcast(tripId, 'collab:note:deleted', { noteId }); + this.guards.safeBroadcast(tripId, 'collab:note:deleted', { noteId }); return ok({ success: true }); } @@ -189,9 +191,9 @@ export class CollabMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); const poll = this.collab.createPoll(tripId, ctx.userId, { question, options, multiple, deadline }); - safeBroadcast(tripId, 'collab:poll:created', { poll }); + this.guards.safeBroadcast(tripId, 'collab:poll:created', { poll }); return ok({ poll }); } @@ -210,10 +212,10 @@ export class CollabMcp { async voteCollabPoll({ tripId, pollId, optionIndex }: { tripId: number; pollId: number; optionIndex: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); const result = this.collab.votePoll(tripId, pollId, ctx.userId, optionIndex); if (result.error) return errorResult(result.error); - safeBroadcast(tripId, 'collab:poll:voted', { poll: result.poll }); + this.guards.safeBroadcast(tripId, 'collab:poll:voted', { poll: result.poll }); return ok({ poll: result.poll }); } @@ -231,10 +233,10 @@ export class CollabMcp { async closeCollabPoll({ tripId, pollId }: { tripId: number; pollId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); const poll = this.collab.closePoll(tripId, pollId); if (!poll) return errorResult('Poll not found.'); - safeBroadcast(tripId, 'collab:poll:closed', { poll }); + this.guards.safeBroadcast(tripId, 'collab:poll:closed', { poll }); return ok({ poll }); } @@ -252,10 +254,10 @@ export class CollabMcp { async deleteCollabPoll({ tripId, pollId }: { tripId: number; pollId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); const deleted = this.collab.deletePoll(tripId, pollId); if (!deleted) return errorResult('Poll not found.'); - safeBroadcast(tripId, 'collab:poll:deleted', { pollId }); + this.guards.safeBroadcast(tripId, 'collab:poll:deleted', { pollId }); return ok({ success: true }); } @@ -291,10 +293,10 @@ export class CollabMcp { async sendCollabMessage({ tripId, text, replyTo }: { tripId: number; text: string; replyTo?: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); const result = this.collab.createMessage(tripId, ctx.userId, text, replyTo ?? null); if (result.error) return errorResult(result.error); - safeBroadcast(tripId, 'collab:message:created', { message: result.message }); + this.guards.safeBroadcast(tripId, 'collab:message:created', { message: result.message }); return ok({ message: result.message }); } @@ -312,10 +314,10 @@ export class CollabMcp { async deleteCollabMessage({ tripId, messageId }: { tripId: number; messageId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); const result = this.collab.deleteMessage(tripId, messageId, ctx.userId); if (result.error) return errorResult(result.error); - safeBroadcast(tripId, 'collab:message:deleted', { messageId, username: result.username }); + this.guards.safeBroadcast(tripId, 'collab:message:deleted', { messageId, username: result.username }); return ok({ success: true }); } @@ -334,10 +336,10 @@ export class CollabMcp { async reactCollabMessage({ tripId, messageId, emoji }: { tripId: number; messageId: number; emoji: string }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); const result = this.collab.reactMessage(messageId, tripId, ctx.userId, emoji); if (!result.found) return errorResult('Message not found.'); - safeBroadcast(tripId, 'collab:message:reacted', { messageId, reactions: result.reactions }); + this.guards.safeBroadcast(tripId, 'collab:message:reacted', { messageId, reactions: result.reactions }); return ok({ reactions: result.reactions }); } diff --git a/server/src/nest/collab/collab.module.ts b/server/src/nest/collab/collab.module.ts index 19fd3f16e..a8f3b440d 100644 --- a/server/src/nest/collab/collab.module.ts +++ b/server/src/nest/collab/collab.module.ts @@ -9,9 +9,10 @@ import { PermissionsModule } from '../permissions/permissions.module'; import { AuthModule } from '../auth/auth.module'; import { AddonsModule } from '../addons/addons.module'; import { NotificationsModule } from '../notifications/notifications.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; @Module({ - imports: [NotificationsModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], + imports: [McpSharedModule, NotificationsModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], controllers: [CollabController], providers: [CollabService, CollabMcp, CollabRpc], // For in-container consumers (CollabRpc). diff --git a/server/src/nest/day-notes/day-notes.mcp.ts b/server/src/nest/day-notes/day-notes.mcp.ts index e5d36a751..5370cefab 100644 --- a/server/src/nest/day-notes/day-notes.mcp.ts +++ b/server/src/nest/day-notes/day-notes.mcp.ts @@ -3,9 +3,10 @@ import { TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { DayNotesService } from './day-notes.service'; function parseId(value: string | string[]): number | null { @@ -25,7 +26,11 @@ function parseId(value: string | string[]): number | null { */ @McpController() export class DayNotesMcp { - constructor(private readonly notes: DayNotesService, private readonly auth: AuthService) {} + constructor( + private readonly notes: DayNotesService, + private readonly auth: AuthService, + private readonly guards: McpToolGuardsService, + ) {} @Tool({ name: 'create_day_note', @@ -46,10 +51,10 @@ export class DayNotesMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.notes.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.notes.dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; const note = this.notes.create(dayId, tripId, text, time, icon); - safeBroadcast(tripId, 'dayNote:created', { dayId, note }); + this.guards.safeBroadcast(tripId, 'dayNote:created', { dayId, note }); return ok({ note }); } @@ -73,11 +78,11 @@ export class DayNotesMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.notes.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const existing = this.notes.getNote(noteId, dayId, tripId); if (!existing) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; const note = this.notes.update(noteId, existing, { text, time: time !== undefined ? time : undefined, icon }); - safeBroadcast(tripId, 'dayNote:updated', { dayId, note }); + this.guards.safeBroadcast(tripId, 'dayNote:updated', { dayId, note }); return ok({ note }); } @@ -95,11 +100,11 @@ export class DayNotesMcp { async deleteDayNote({ tripId, dayId, noteId }: { tripId: number; dayId: number; noteId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.notes.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const note = this.notes.getNote(noteId, dayId, tripId); if (!note) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; this.notes.remove(noteId); - safeBroadcast(tripId, 'dayNote:deleted', { noteId, dayId }); + this.guards.safeBroadcast(tripId, 'dayNote:deleted', { noteId, dayId }); return ok({ success: true }); } diff --git a/server/src/nest/day-notes/day-notes.module.ts b/server/src/nest/day-notes/day-notes.module.ts index 8a3f55446..c2ff48110 100644 --- a/server/src/nest/day-notes/day-notes.module.ts +++ b/server/src/nest/day-notes/day-notes.module.ts @@ -7,6 +7,7 @@ import { PermissionsModule } from '../permissions/permissions.module'; import { AuthModule } from '../auth/auth.module'; import { RealtimeModule } from '../realtime/realtime.module'; import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** * Day notes. Its own domain rather than a second file set inside days/, which @@ -20,7 +21,7 @@ import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; * for DayNotesRpc. */ @Module({ - imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule], + imports: [McpSharedModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule], controllers: [DayNotesController], providers: [DayNotesService, DayNotesMcp, DayNotesRpc], exports: [DayNotesService], diff --git a/server/src/nest/days/days.mcp.ts b/server/src/nest/days/days.mcp.ts index d3f78ce01..06959c41e 100644 --- a/server/src/nest/days/days.mcp.ts +++ b/server/src/nest/days/days.mcp.ts @@ -3,9 +3,10 @@ import { TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { DaysService } from './days.service'; function parseId(value: string | string[]): number | null { @@ -30,6 +31,7 @@ export class DaysMcp { constructor( private readonly days: DaysService, private readonly auth: AuthService, + private readonly guards: McpToolGuardsService, ) {} @Tool({ @@ -49,11 +51,11 @@ export class DaysMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.days.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const current = this.days.getDay(dayId, tripId); if (!current) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; const updated = this.days.update(dayId, current, { title }); - safeBroadcast(tripId, 'day:updated', { day: updated }); + this.guards.safeBroadcast(tripId, 'day:updated', { day: updated }); return ok({ day: updated }); } @@ -74,9 +76,9 @@ export class DaysMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.days.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const day = this.days.create(tripId, date, notes); - safeBroadcast(tripId, 'day:created', { day }); + this.guards.safeBroadcast(tripId, 'day:created', { day }); return ok({ day }); } @@ -93,12 +95,12 @@ export class DaysMcp { async deleteDay({ tripId, dayId }: { tripId: number; dayId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.days.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.days.getDay(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; this.days.remove(dayId); // REST parity shape ({ dayId }) — the client reads payload.dayId, so the { id } // variant never removed the day from collaborator screens. - safeBroadcast(tripId, 'day:deleted', { dayId }); + this.guards.safeBroadcast(tripId, 'day:deleted', { dayId }); return ok({ success: true }); } diff --git a/server/src/nest/days/days.module.ts b/server/src/nest/days/days.module.ts index ab25c1984..6eb6b9c1b 100644 --- a/server/src/nest/days/days.module.ts +++ b/server/src/nest/days/days.module.ts @@ -9,6 +9,7 @@ import { PermissionsModule } from '../permissions/permissions.module'; import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { PlacesModule } from '../places/places.module'; import { AuthModule } from '../auth/auth.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** * Days (S6 — Phase 2 trip sub-domain), mounted at /api/trips/:tripId/days. @@ -20,7 +21,7 @@ import { AuthModule } from '../auth/auth.module'; * own domain now (day-notes/). */ @Module({ - imports: [PermissionsModule, QueryHelpersModule, PlacesModule, AuthModule, RealtimeModule, PluginGuardsModule], + imports: [McpSharedModule, PermissionsModule, QueryHelpersModule, PlacesModule, AuthModule, RealtimeModule, PluginGuardsModule], controllers: [DaysController], providers: [DaysService, DaysMcp, DaysRpc], exports: [DaysService], diff --git a/server/src/nest/mcp-shared/mcp-shared.module.ts b/server/src/nest/mcp-shared/mcp-shared.module.ts new file mode 100644 index 000000000..e384f5c2a --- /dev/null +++ b/server/src/nest/mcp-shared/mcp-shared.module.ts @@ -0,0 +1,16 @@ +import { Module } from '@nestjs/common'; +import { McpToolGuardsService } from './mcp-tool-guards.service'; +import { PermissionsModule } from '../permissions/permissions.module'; + +/** + * Shared guards for the @McpController domain classes. Deliberately NOT + * @Global (the addons/permissions precedent): every consumer imports it + * explicitly, so e2e TestingModules resolve it transitively and the module + * graph stays honest. Database/Realtime come from their @Global modules. + */ +@Module({ + imports: [PermissionsModule], + providers: [McpToolGuardsService], + exports: [McpToolGuardsService], +}) +export class McpSharedModule {} diff --git a/server/src/nest/mcp-shared/mcp-tool-guards.service.ts b/server/src/nest/mcp-shared/mcp-tool-guards.service.ts new file mode 100644 index 000000000..1324cc2ad --- /dev/null +++ b/server/src/nest/mcp-shared/mcp-tool-guards.service.ts @@ -0,0 +1,55 @@ +import { Injectable } from '@nestjs/common'; +import { DatabaseService } from '../database/database.service'; +import { PermissionsService } from '../permissions/permissions.service'; +import { RealtimeService } from '../realtime/realtime.service'; + +/** + * The impure MCP tool guards that used to live as plain functions in + * src/mcp/tools/_shared.ts, reaching the db Proxy, permissions.bridge and the + * src/websocket stub as module globals. The @McpController domain classes are + * ordinary Nest providers, so they inject this instead. The pure result + * helpers (noAccess/permissionDenied/adminRequired/MAX_MCP_TRIP_DAYS and the + * @trek/nest-mcp re-exports) stay in _shared.ts — they carry no dependencies. + */ +@Injectable() +export class McpToolGuardsService { + constructor( + private readonly db: DatabaseService, + private readonly permissions: PermissionsService, + private readonly realtime: RealtimeService, + ) {} + + safeBroadcast(tripId: number, event: string, payload: Record): void { + try { + // Legacy MCP call sites pass event names as plain strings; route through + // the facade's implementation signature (its typed overloads are for new + // call sites). Runtime is a pure pass-through to src/websocket's + // broadcast, so the 100+ per-file vi.mock stubs keep intercepting. + (this.realtime.broadcast as (t: number | string, e: string, p: Record) => void)( + tripId, event, { ...payload, _source: 'mcp' }, + ); + } catch (err) { + console.error(`[MCP] broadcast failed for ${event}:`, (err as Error | undefined)?.message ?? err); + } + } + + /** + * 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`. + */ + hasTripPermission(action: string, tripId: number | string, userId: number): boolean { + const trip = this.db.get<{ user_id?: number }>('SELECT user_id FROM trips WHERE id = ?', tripId); + if (!trip) return false; + const userRow = this.db.get<{ role?: string }>('SELECT role FROM users WHERE id = ?', userId); + const tripOwnerId = typeof trip.user_id === 'number' ? trip.user_id : null; + return this.permissions.checkPermission(action, userRow?.role ?? 'user', tripOwnerId, userId, tripOwnerId !== userId); + } + + /** True when the user has the global admin role (mirrors REST `user.role === 'admin'` gates). */ + isAdminUser(userId: number): boolean { + const userRow = this.db.get<{ role?: string }>('SELECT role FROM users WHERE id = ?', userId); + return userRow?.role === 'admin'; + } +} diff --git a/server/src/nest/packing/packing.mcp.ts b/server/src/nest/packing/packing.mcp.ts index 9df1e3e7a..086f77391 100644 --- a/server/src/nest/packing/packing.mcp.ts +++ b/server/src/nest/packing/packing.mcp.ts @@ -4,10 +4,11 @@ import { TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, errorResult, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; import { ADDON_IDS } from '../../addons'; -import { safeBroadcast, noAccess, hasTripPermission, permissionDenied, isAdminUser, adminRequired } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied, adminRequired } from '../../mcp/tools/_shared'; import { PackingService } from './packing.service'; import { addonGate } from '../addons/addon-gate'; import { AddonsService } from '../addons/addons.service'; @@ -38,6 +39,7 @@ export class PackingMcp { private readonly packing: PackingService, private readonly auth: AuthService, readonly addons: AddonsService, + private readonly guards: McpToolGuardsService, ) {} // --- PACKING --- @@ -57,9 +59,9 @@ export class PackingMcp { async createPackingItem({ tripId, name, category }: { tripId: number; name: string; category?: string }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const item = this.packing.createItem(tripId, { name, category: category || 'General' }, ctx.userId); - safeBroadcast(tripId, 'packing:created', { item }); + this.guards.safeBroadcast(tripId, 'packing:created', { item }); return ok({ item }); } @@ -78,10 +80,10 @@ export class PackingMcp { async togglePackingItem({ tripId, itemId, checked }: { tripId: number; itemId: number; checked: boolean }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const item = this.packing.updateItem(tripId, itemId, { checked: checked ? 1 : 0 }, ['checked']); if (!item) return errorResult('Packing item not found.'); - safeBroadcast(tripId, 'packing:updated', { item }); + this.guards.safeBroadcast(tripId, 'packing:updated', { item }); return ok({ item }); } @@ -99,10 +101,10 @@ export class PackingMcp { async deletePackingItem({ tripId, itemId }: { tripId: number; itemId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const deleted = this.packing.deleteItem(tripId, itemId); if (!deleted) return errorResult('Packing item not found.'); - safeBroadcast(tripId, 'packing:deleted', { itemId }); + this.guards.safeBroadcast(tripId, 'packing:deleted', { itemId }); return ok({ success: true }); } @@ -124,11 +126,11 @@ export class PackingMcp { async updatePackingItem({ tripId, itemId, name, category }: { tripId: number; itemId: number; name?: string; category?: string }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const bodyKeys = ['name', 'category'].filter(k => k === 'name' ? name !== undefined : category !== undefined); const item = this.packing.updateItem(tripId, itemId, { name, category }, bodyKeys); if (!item) return errorResult('Packing item not found.'); - safeBroadcast(tripId, 'packing:updated', { item }); + this.guards.safeBroadcast(tripId, 'packing:updated', { item }); return ok({ item }); } @@ -148,9 +150,9 @@ export class PackingMcp { async reorderPackingItems({ tripId, orderedIds }: { tripId: number; orderedIds: number[] }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); this.packing.reorderItems(tripId, orderedIds); - safeBroadcast(tripId, 'packing:reordered', { orderedIds }); + this.guards.safeBroadcast(tripId, 'packing:reordered', { orderedIds }); return ok({ success: true }); } @@ -185,11 +187,11 @@ export class PackingMcp { async createPackingBag({ tripId, name, color }: { tripId: number; name: string; color?: string }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); // createBag returns a bare row; hydrate with the empty members array that // listBags and the schema always carry, so the client/AI consumer matches. const bag = { ...(this.packing.createBag(tripId, { name, color }) as object), members: [] }; - safeBroadcast(tripId, 'packing:bag-created', { bag }); + this.guards.safeBroadcast(tripId, 'packing:bag-created', { bag }); return ok({ bag }); } @@ -209,7 +211,7 @@ export class PackingMcp { async updatePackingBag({ tripId, bagId, name, color }: { tripId: number; bagId: number; name?: string; color?: string }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const fields: Record = {}; const bodyKeys: string[] = []; if (name !== undefined) { fields.name = name; bodyKeys.push('name'); } @@ -218,7 +220,7 @@ export class PackingMcp { if (!updated) return errorResult('Bag not found.'); // Hydrate with the members array (matches create_packing_bag, listBags, and the schema). const bag = this.packing.listBags(tripId).find(b => b.id === (updated as { id: number }).id) ?? { ...(updated as object), members: [] }; - safeBroadcast(tripId, 'packing:bag-updated', { bag }); + this.guards.safeBroadcast(tripId, 'packing:bag-updated', { bag }); return ok({ bag }); } @@ -236,11 +238,11 @@ export class PackingMcp { async deletePackingBag({ tripId, bagId }: { tripId: number; bagId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); this.packing.deleteBag(tripId, bagId); // { bagId } matches the REST route and the plugin host (the legacy // registrar's { id } was the odd one out). - safeBroadcast(tripId, 'packing:bag-deleted', { bagId }); + this.guards.safeBroadcast(tripId, 'packing:bag-deleted', { bagId }); return ok({ success: true }); } @@ -259,10 +261,10 @@ export class PackingMcp { async setBagMembers({ tripId, bagId, userIds }: { tripId: number; bagId: number; userIds: number[] }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const members = this.packing.setBagMembers(tripId, bagId, userIds); if (!members) return errorResult('Bag not found.'); - safeBroadcast(tripId, 'packing:bag-members-updated', { bagId, members }); + this.guards.safeBroadcast(tripId, 'packing:bag-members-updated', { bagId, members }); return ok({ members }); } @@ -297,9 +299,9 @@ export class PackingMcp { async setPackingCategoryAssignees({ tripId, categoryName, userIds }: { tripId: number; categoryName: string; userIds: number[] }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const assignees = this.packing.updateCategoryAssignees(tripId, categoryName, userIds); - safeBroadcast(tripId, 'packing:assignees', { category: categoryName, assignees }); + this.guards.safeBroadcast(tripId, 'packing:assignees', { category: categoryName, assignees }); return ok({ assignees }); } @@ -317,10 +319,10 @@ export class PackingMcp { async applyPackingTemplate({ tripId, templateId }: { tripId: number; templateId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const items = this.packing.applyTemplate(tripId, templateId); if (items === null) return errorResult('Template not found.'); - safeBroadcast(tripId, 'packing:template-applied', { items }); + this.guards.safeBroadcast(tripId, 'packing:template-applied', { items }); return ok({ items, count: items.length }); } @@ -353,9 +355,9 @@ export class PackingMcp { async savePackingTemplate({ tripId, templateName }: { tripId: number; templateName: string }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); // Templates are global; the REST route restricts saving to admins. Match it. - if (!isAdminUser(ctx.userId)) return adminRequired(); + if (!this.guards.isAdminUser(ctx.userId)) return adminRequired(); const template = this.packing.saveAsTemplate(tripId, ctx.userId, templateName); if (!template) return errorResult('Nothing to save — the packing list is empty.'); return ok({ template }); @@ -374,7 +376,7 @@ export class PackingMcp { async deletePackingTemplate({ templateId }: { templateId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); // Templates are global; the REST route restricts management to admins. Match it. - if (!isAdminUser(ctx.userId)) return adminRequired(); + if (!this.guards.isAdminUser(ctx.userId)) return adminRequired(); const result = this.packing.deletePackingTemplate(String(templateId)); if ('error' in result) return errorResult(result.error); return ok({ success: true, name: result.name }); @@ -404,9 +406,9 @@ export class PackingMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const created = this.packing.bulkImport(tripId, items, ctx.userId); - for (const item of created) safeBroadcast(tripId, 'packing:created', { item }); + for (const item of created) this.guards.safeBroadcast(tripId, 'packing:created', { item }); return ok({ items: created, count: created.length }); } diff --git a/server/src/nest/packing/packing.module.ts b/server/src/nest/packing/packing.module.ts index 42ed54fa7..54f5d1ace 100644 --- a/server/src/nest/packing/packing.module.ts +++ b/server/src/nest/packing/packing.module.ts @@ -11,12 +11,13 @@ import { AuditModule } from '../audit/audit.module'; import { AuthModule } from '../auth/auth.module'; import { AddonsModule } from '../addons/addons.module'; import { NotificationsModule } from '../notifications/notifications.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** Packing domain (S2 — Phase 2 trip sub-domain). Registered in AppModule. * Exports PackingService for in-container consumers (TripsService bundle, * PackingRpc). */ @Module({ - imports: [NotificationsModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule, AuditModule], + imports: [McpSharedModule, NotificationsModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule, AuditModule], controllers: [PackingController, AdminPackingTemplatesController], providers: [PackingService, PackingMcp, PackingRpc], exports: [PackingService], diff --git a/server/src/nest/permissions/permissions.bridge.ts b/server/src/nest/permissions/permissions.bridge.ts deleted file mode 100644 index f19ad0257..000000000 --- a/server/src/nest/permissions/permissions.bridge.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { db } from '../../db/database'; -import { DatabaseService } from '../database/database.service'; -import { PermissionsService } from './permissions.service'; - -/** - * Non-Nest entry point for the permissions domain — pinned by the MCP - * transport's hasTripPermission helper (src/mcp/tools/_shared.ts), which the - * pre-app.init() MCP/OAuth mount evaluates before the container exists. - * Inside the container, inject PermissionsService instead. Delete this file - * when the MCP/OAuth mount moves behind the container. - * - * The permissions cache is module-scoped in permissions-cache.ts, so this - * instance and the container singleton share one cache (backup restores - * invalidate it through that module directly). - * - * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto - * the shared better-sqlite3 singleton. - */ -const permissions = new PermissionsService(new DatabaseService(db)); - -export function checkPermission( - actionKey: string, - userRole: string, - tripUserId: number | null, - userId: number, - isMember: boolean -): boolean { - return permissions.checkPermission(actionKey, userRole, tripUserId, userId, isMember); -} diff --git a/server/src/nest/places/places.mcp.ts b/server/src/nest/places/places.mcp.ts index dff83a539..79a3a943b 100644 --- a/server/src/nest/places/places.mcp.ts +++ b/server/src/nest/places/places.mcp.ts @@ -4,11 +4,12 @@ import { TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; import { AssignmentsService } from '../assignments/assignments.service'; import { JourneyDomainService } from '../journey/journey-domain.service'; -import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { DatabaseService } from '../database/database.service'; import { MapsService } from '../maps/maps.service'; import { PlacesService } from './places.service'; @@ -44,6 +45,7 @@ export class PlacesMcp { private readonly auth: AuthService, private readonly journey: JourneyDomainService, private readonly assignments: AssignmentsService, + private readonly guards: McpToolGuardsService, ) {} @Tool({ @@ -79,9 +81,9 @@ export class PlacesMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); const place = this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, notes, website, phone, price, currency }); - safeBroadcast(tripId, 'place:created', { place }); + this.guards.safeBroadcast(tripId, 'place:created', { place }); return ok({ place }); } @@ -121,7 +123,7 @@ export class PlacesMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.assignments.dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; try { const result = this.db.transaction(() => { @@ -129,8 +131,8 @@ export class PlacesMcp { const assignment = this.assignments.createAssignment(dayId, place.id, assignment_notes ?? null); return { place, assignment }; }); - safeBroadcast(tripId, 'place:created', { place: result.place }); - safeBroadcast(tripId, 'assignment:created', { assignment: result.assignment }); + this.guards.safeBroadcast(tripId, 'place:created', { place: result.place }); + this.guards.safeBroadcast(tripId, 'assignment:created', { assignment: result.assignment }); try { this.journey.reconcileTripSkeletons(tripId); } catch { /* non-fatal */ } return ok(result); } catch { @@ -178,10 +180,10 @@ export class PlacesMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); const place = this.places.update(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, google_ftid }); if (!place) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; - safeBroadcast(tripId, 'place:updated', { place }); + this.guards.safeBroadcast(tripId, 'place:updated', { place }); return ok({ place }); } @@ -205,7 +207,7 @@ export class PlacesMcp { if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); const place = this.places.rate(String(tripId), String(placeId), ctx.userId, rating ?? null); if (!place) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; - safeBroadcast(tripId, 'place:updated', { place }); + this.guards.safeBroadcast(tripId, 'place:updated', { place }); return ok({ place }); } @@ -222,7 +224,7 @@ export class PlacesMcp { async deletePlace({ tripId, placeId }: { tripId: number; placeId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); // Scope the id to the trip before the hook: onPlaceDeleted keys on the place // id alone, so a foreign id would detach that trip's journey entries even // though the delete below refuses it. @@ -232,7 +234,7 @@ export class PlacesMcp { try { this.journey.onPlaceDeleted(placeId); } catch { /* non-fatal */ } // sync journeys before the row is gone const deleted = this.places.remove(String(tripId), String(placeId)); if (!deleted) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; - safeBroadcast(tripId, 'place:deleted', { placeId }); + this.guards.safeBroadcast(tripId, 'place:deleted', { placeId }); return ok({ success: true }); } @@ -301,7 +303,7 @@ export class PlacesMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); const result = source === 'google-list' ? await this.places.importGoogleList(String(tripId), url) @@ -312,7 +314,7 @@ export class PlacesMcp { } for (const place of result.places) { - safeBroadcast(tripId, 'place:created', { place }); + this.guards.safeBroadcast(tripId, 'place:created', { place }); } return ok({ places: result.places, count: result.places.length, listName: result.listName, skipped: result.skipped }); } @@ -330,7 +332,7 @@ export class PlacesMcp { async bulkDeletePlaces({ tripId, placeIds }: { tripId: number; placeIds: number[] }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); // Trip-scoped, and ahead of the DELETE: journey_entries.source_place_id is // ON DELETE SET NULL, so a hook that ran afterwards found nothing left to @@ -339,7 +341,7 @@ export class PlacesMcp { try { this.journey.onPlaceDeleted(id); } catch { /* non-fatal */ } } const deleted = this.places.removeMany(String(tripId), placeIds); - for (const id of deleted) safeBroadcast(tripId, 'place:deleted', { placeId: id }); + for (const id of deleted) this.guards.safeBroadcast(tripId, 'place:deleted', { placeId: id }); return ok({ deleted, count: deleted.length }); } @@ -375,7 +377,7 @@ export class PlacesMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); const fields = { category_id, price, currency, transport_mode, place_time, end_time, duration_minutes, notes, website, phone, description }; if (Object.values(fields).every(v => v === undefined)) { @@ -383,7 +385,7 @@ export class PlacesMcp { } const updated = this.places.updateMany(String(tripId), placeIds, fields); - for (const place of updated) safeBroadcast(tripId, 'place:updated', { place }); + for (const place of updated) this.guards.safeBroadcast(tripId, 'place:updated', { place }); return ok({ count: updated.length, updatedIds: updated.map(p => p.id), skipped: placeIds.length - updated.length }); } diff --git a/server/src/nest/places/places.module.ts b/server/src/nest/places/places.module.ts index 5cb78a8c4..07725ff76 100644 --- a/server/src/nest/places/places.module.ts +++ b/server/src/nest/places/places.module.ts @@ -14,6 +14,7 @@ import { PlacePhotosModule } from '../place-photos/place-photos.module'; import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { MapsModule } from '../maps/maps.module'; import { AuthModule } from '../auth/auth.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** * Places domain (S8 — Phase 2 trip sub-domain). Depends on L4 Categories + L5 @@ -24,7 +25,7 @@ import { AuthModule } from '../auth/auth.module'; * is no places.bridge.ts: nothing outside the container consumes this domain. */ @Module({ - imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule, PlacePhotosModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule, AssignmentsDomainModule], + imports: [McpSharedModule, PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule, PlacePhotosModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule, AssignmentsDomainModule], controllers: [PlacesController], providers: [PlacesService, PlacesMcp, PlacesRpc], exports: [PlacesService], diff --git a/server/src/nest/reservations/reservations.mcp.ts b/server/src/nest/reservations/reservations.mcp.ts index 5328ba59e..04d6f21d0 100644 --- a/server/src/nest/reservations/reservations.mcp.ts +++ b/server/src/nest/reservations/reservations.mcp.ts @@ -3,10 +3,11 @@ import { TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, errorResult, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; import { BudgetService } from '../budget/budget.service'; -import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { ReservationsService } from './reservations.service'; import { DaysService } from '../days/days.service'; import { findByIata } from '../airports/airports.data'; @@ -91,6 +92,7 @@ export class ReservationsMcp { // Appended, not inserted: the hand-wired MCP harnesses build this // positionally. private readonly assignments: AssignmentsService, + private readonly guards: McpToolGuardsService, ) {} @Tool({ @@ -128,7 +130,7 @@ export class ReservationsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); // Validate that all referenced IDs belong to this trip if (day_id && !this.days.getDay(day_id, tripId)) @@ -156,7 +158,7 @@ export class ReservationsMcp { }); if (accommodationCreated) { - safeBroadcast(tripId, 'accommodation:created', {}); + this.guards.safeBroadcast(tripId, 'accommodation:created', {}); } if (price != null && price > 0) { @@ -165,10 +167,10 @@ export class ReservationsMcp { category: budget_category || type, total_price: price, }); - safeBroadcast(tripId, 'budget:created', { item }); + this.guards.safeBroadcast(tripId, 'budget:created', { item }); } - safeBroadcast(tripId, 'reservation:created', { reservation }); + this.guards.safeBroadcast(tripId, 'reservation:created', { reservation }); return ok({ reservation }); } @@ -202,7 +204,7 @@ export class ReservationsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); const existing = this.reservations.getReservation(reservationId, tripId); if (!existing) return errorResult('Reservation not found.'); @@ -216,7 +218,7 @@ export class ReservationsMcp { place_id: place_id !== undefined ? place_id ?? undefined : undefined, assignment_id: assignment_id !== undefined ? assignment_id ?? undefined : undefined, }, existing); - safeBroadcast(tripId, 'reservation:updated', { reservation }); + this.guards.safeBroadcast(tripId, 'reservation:updated', { reservation }); return ok({ reservation }); } @@ -233,13 +235,13 @@ export class ReservationsMcp { async deleteReservation({ tripId, reservationId }: { tripId: number; reservationId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); const { deleted, accommodationDeleted } = this.reservations.remove(reservationId, tripId); if (!deleted) return errorResult('Reservation not found.'); if (accommodationDeleted) { - safeBroadcast(tripId, 'accommodation:deleted', { accommodationId: deleted.accommodation_id }); + this.guards.safeBroadcast(tripId, 'accommodation:deleted', { accommodationId: deleted.accommodation_id }); } - safeBroadcast(tripId, 'reservation:deleted', { reservationId }); + this.guards.safeBroadcast(tripId, 'reservation:deleted', { reservationId }); return ok({ success: true }); } @@ -263,9 +265,9 @@ export class ReservationsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); this.reservations.updatePositions(tripId, positions, dayId); - safeBroadcast(tripId, 'reservation:positions', { positions, dayId }); + this.guards.safeBroadcast(tripId, 'reservation:positions', { positions, dayId }); return ok({ success: true }); } @@ -293,7 +295,7 @@ export class ReservationsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); const current = this.reservations.getReservation(reservationId, tripId); if (!current) return errorResult('Reservation not found.'); if (current.type !== 'hotel') return errorResult('Reservation is not of type hotel.'); @@ -313,8 +315,8 @@ export class ReservationsMcp { create_accommodation: { place_id, start_day_id, end_day_id, check_in: check_in || undefined, check_out: check_out || undefined }, }, current); - safeBroadcast(tripId, isNewAccommodation ? 'accommodation:created' : 'accommodation:updated', {}); - safeBroadcast(tripId, 'reservation:updated', { reservation }); + this.guards.safeBroadcast(tripId, isNewAccommodation ? 'accommodation:created' : 'accommodation:updated', {}); + this.guards.safeBroadcast(tripId, 'reservation:updated', { reservation }); return ok({ reservation, accommodation_id: reservation?.accommodation_id ?? null }); } @@ -392,7 +394,7 @@ export class ReservationsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); if (start_day_id && !this.days.getDay(start_day_id, tripId)) return errorResult('start_day_id does not belong to this trip.'); @@ -427,10 +429,10 @@ export class ReservationsMcp { category: budget_category || type, total_price: price, }); - safeBroadcast(tripId, 'budget:created', { item }); + this.guards.safeBroadcast(tripId, 'budget:created', { item }); } - safeBroadcast(tripId, 'reservation:created', { reservation }); + this.guards.safeBroadcast(tripId, 'reservation:created', { reservation }); return ok({ reservation }); } @@ -467,7 +469,7 @@ export class ReservationsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); const existing = this.reservations.getReservation(reservationId, tripId); if (!existing) return errorResult('Transport not found.'); @@ -503,7 +505,7 @@ export class ReservationsMcp { endpoints: resolvedEndpoints, needs_review, }, existing); - safeBroadcast(tripId, 'reservation:updated', { reservation }); + this.guards.safeBroadcast(tripId, 'reservation:updated', { reservation }); return ok({ reservation }); } @@ -520,10 +522,10 @@ export class ReservationsMcp { async deleteTransport({ tripId, reservationId }: { tripId: number; reservationId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); const { deleted } = this.reservations.remove(reservationId, tripId); if (!deleted) return errorResult('Transport not found.'); - safeBroadcast(tripId, 'reservation:deleted', { reservationId }); + this.guards.safeBroadcast(tripId, 'reservation:deleted', { reservationId }); return ok({ success: true }); } } diff --git a/server/src/nest/reservations/reservations.module.ts b/server/src/nest/reservations/reservations.module.ts index 4701863e2..3aa7d3df3 100644 --- a/server/src/nest/reservations/reservations.module.ts +++ b/server/src/nest/reservations/reservations.module.ts @@ -14,6 +14,7 @@ import { UpcomingReservationsController } from './upcoming-reservations.controll import { AuthModule } from '../auth/auth.module'; import { AssignmentsModule } from '../assignments/assignments.module'; import { NotificationsModule } from '../notifications/notifications.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** * Reservations + accommodations domain (S5 — Phase 2 trip sub-domain). @@ -24,7 +25,7 @@ import { NotificationsModule } from '../notifications/notifications.module'; @Module({ // DaysModule: ReservationsMcp injects DaysService for its nine getDay calls. // BudgetModule: ReservationsService + ReservationsMcp inject BudgetService (budget-sync seam). - imports: [NotificationsModule, DaysModule, AssignmentsModule, PermissionsModule, BudgetModule, AuthModule, RealtimeModule, PluginGuardsModule, ReservationsReadModule, AirtrailCoreModule], + imports: [McpSharedModule, NotificationsModule, DaysModule, AssignmentsModule, PermissionsModule, BudgetModule, AuthModule, RealtimeModule, PluginGuardsModule, ReservationsReadModule, AirtrailCoreModule], controllers: [ReservationsController, UpcomingReservationsController], providers: [ReservationsService, ReservationsMcp, ReservationsRpc], // For in-container consumers (ReservationsRpc, TripsService, BookingImportService). diff --git a/server/src/nest/share/share.mcp.ts b/server/src/nest/share/share.mcp.ts index 9368cfef1..0ddefc3c9 100644 --- a/server/src/nest/share/share.mcp.ts +++ b/server/src/nest/share/share.mcp.ts @@ -3,9 +3,10 @@ import { TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, demoDenied, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { canShareTrips } from '../../mcp/scopes'; import { ShareService } from './share.service'; @@ -19,7 +20,11 @@ import { ShareService } from './share.service'; */ @McpController() export class ShareMcp { - constructor(private readonly share: ShareService, private readonly auth: AuthService) {} + constructor( + private readonly share: ShareService, + private readonly auth: AuthService, + private readonly guards: McpToolGuardsService, + ) {} @Tool({ name: 'get_share_link', @@ -36,7 +41,7 @@ export class ShareMcp { // token itself, and a token is an anonymous copy of the trip. Leaving this // one on membership alone would just move the same hole to MCP. if (!this.share.verifyTripAccess(String(tripId), ctx.userId)) return noAccess(); - if (!hasTripPermission('share_manage', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('share_manage', tripId, ctx.userId)) return permissionDenied(); const link = this.share.get(String(tripId)); return ok({ link }); } @@ -63,7 +68,7 @@ export class ShareMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.share.verifyTripAccess(String(tripId), ctx.userId)) return noAccess(); - if (!hasTripPermission('share_manage', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('share_manage', tripId, ctx.userId)) return permissionDenied(); // The zod .default()s above fill omitted flags, and ShareService applies // the same defaults again for undefined — no re-defaulting needed here. const { token, created } = this.share.createOrUpdate(String(tripId), ctx.userId, { @@ -84,7 +89,7 @@ export class ShareMcp { async deleteShareLink({ tripId }: { tripId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.share.verifyTripAccess(String(tripId), ctx.userId)) return noAccess(); - if (!hasTripPermission('share_manage', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('share_manage', tripId, ctx.userId)) return permissionDenied(); this.share.remove(String(tripId)); return ok({ success: true }); } diff --git a/server/src/nest/share/share.module.ts b/server/src/nest/share/share.module.ts index 9b8d0e01a..12e5afc6c 100644 --- a/server/src/nest/share/share.module.ts +++ b/server/src/nest/share/share.module.ts @@ -7,9 +7,10 @@ import { PermissionsModule } from '../permissions/permissions.module'; import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { PlacePhotosModule } from '../place-photos/place-photos.module'; import { AuthModule } from '../auth/auth.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; @Module({ - imports: [SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule, PlacePhotosModule], + imports: [McpSharedModule, SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule, PlacePhotosModule], controllers: [TripShareController, SharedController], providers: [ShareService, ShareMcp], }) diff --git a/server/src/nest/todo/todo.mcp.ts b/server/src/nest/todo/todo.mcp.ts index a98e66f02..be1cb5e20 100644 --- a/server/src/nest/todo/todo.mcp.ts +++ b/server/src/nest/todo/todo.mcp.ts @@ -4,10 +4,11 @@ import { TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, errorResult, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; import { ADDON_IDS } from '../../addons'; -import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { TodoService } from './todo.service'; import { addonGate } from '../addons/addon-gate'; import { AddonsService } from '../addons/addons.service'; @@ -35,6 +36,7 @@ export class TodoMcp { private readonly todos: TodoService, private readonly auth: AuthService, readonly addons: AddonsService, + private readonly guards: McpToolGuardsService, ) {} @Tool({ @@ -77,9 +79,9 @@ export class TodoMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const item = this.todos.createItem(tripId, { name, category, due_date, description, assigned_user_id, priority }); - safeBroadcast(tripId, 'todo:created', { item }); + this.guards.safeBroadcast(tripId, 'todo:created', { item }); return ok({ item }); } @@ -108,7 +110,7 @@ export class TodoMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); // Build bodyKeys to signal which nullable fields were explicitly provided const bodyKeys: string[] = []; if (due_date !== undefined) bodyKeys.push('due_date'); @@ -117,7 +119,7 @@ export class TodoMcp { if (priority !== undefined) bodyKeys.push('priority'); const item = this.todos.updateItem(tripId, itemId, { name, category, due_date, description, assigned_user_id, priority }, bodyKeys); if (!item) return errorResult('To-do item not found.'); - safeBroadcast(tripId, 'todo:updated', { item }); + this.guards.safeBroadcast(tripId, 'todo:updated', { item }); return ok({ item }); } @@ -136,10 +138,10 @@ export class TodoMcp { async toggleTodo({ tripId, itemId, checked }: { tripId: number; itemId: number; checked: boolean }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const item = this.todos.updateItem(tripId, itemId, { checked: checked ? 1 : 0 }, []); if (!item) return errorResult('To-do item not found.'); - safeBroadcast(tripId, 'todo:updated', { item }); + this.guards.safeBroadcast(tripId, 'todo:updated', { item }); return ok({ item }); } @@ -157,10 +159,10 @@ export class TodoMcp { async deleteTodo({ tripId, itemId }: { tripId: number; itemId: number }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const deleted = this.todos.deleteItem(tripId, itemId); if (!deleted) return errorResult('To-do item not found.'); - safeBroadcast(tripId, 'todo:deleted', { itemId }); + this.guards.safeBroadcast(tripId, 'todo:deleted', { itemId }); return ok({ success: true }); } @@ -178,7 +180,7 @@ export class TodoMcp { async reorderTodos({ tripId, orderedIds }: { tripId: number; orderedIds: number[] }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); this.todos.reorderItems(tripId, orderedIds); return ok({ success: true }); } @@ -214,9 +216,9 @@ export class TodoMcp { async setTodoCategoryAssignees({ tripId, categoryName, userIds }: { tripId: number; categoryName: string; userIds: number[] }, ctx: McpContext) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); const assignees = this.todos.updateCategoryAssignees(tripId, categoryName, userIds); - safeBroadcast(tripId, 'todo:assignees', { category: categoryName, assignees }); + this.guards.safeBroadcast(tripId, 'todo:assignees', { category: categoryName, assignees }); return ok({ assignees }); } diff --git a/server/src/nest/todo/todo.module.ts b/server/src/nest/todo/todo.module.ts index 00f2b4c90..94d17170e 100644 --- a/server/src/nest/todo/todo.module.ts +++ b/server/src/nest/todo/todo.module.ts @@ -8,11 +8,12 @@ import { RealtimeModule } from '../realtime/realtime.module'; import { PermissionsModule } from '../permissions/permissions.module'; import { AuthModule } from '../auth/auth.module'; import { AddonsModule } from '../addons/addons.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** To-do domain (S3 — Phase 2 trip sub-domain). Registered in AppModule. * Exports TodoService for in-container consumers (TripsService bundle). */ @Module({ - imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], + imports: [McpSharedModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], controllers: [TodoController], providers: [TodoService, TodoMcp, TodoRpc], exports: [TodoService], diff --git a/server/src/nest/transit/transit.mcp.ts b/server/src/nest/transit/transit.mcp.ts index b1b64f824..918926248 100644 --- a/server/src/nest/transit/transit.mcp.ts +++ b/server/src/nest/transit/transit.mcp.ts @@ -3,6 +3,7 @@ import { TOOL_ANNOTATIONS_OPEN_WORLD_NON_IDEMPOTENT, TOOL_ANNOTATIONS_OPEN_WORLD_READONLY, demoDenied, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; import { @@ -13,7 +14,7 @@ import { transitItinerarySchema, transitPlaceSchema, } from './transit-itinerary.helpers'; -import { hasTripPermission, noAccess, permissionDenied, safeBroadcast } from '../../mcp/tools/_shared'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { RateLimitService } from '../common/rate-limit.service'; import { DatabaseService } from '../database/database.service'; import { DaysService } from '../days/days.service'; @@ -64,6 +65,7 @@ export class TransitMcp { private readonly reservations: ReservationsService, private readonly db: DatabaseService, private readonly auth: AuthService, + private readonly guards: McpToolGuardsService, ) {} @Tool({ @@ -183,7 +185,7 @@ export class TransitMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); const day = this.days.getDay(dayId, tripId); if (!day) { return { content: [{ type: 'text' as const, text: 'dayId does not belong to this trip.' }], isError: true }; @@ -246,7 +248,7 @@ export class TransitMcp { endpoints, needs_review: false, }); - safeBroadcast(tripId, 'reservation:created', { reservation }); + this.guards.safeBroadcast(tripId, 'reservation:created', { reservation }); this.reservations.notifyBookingChange(tripId, ctx.userId, reservation.title, reservation.type || ''); return ok({ reservation }); } diff --git a/server/src/nest/transit/transit.module.ts b/server/src/nest/transit/transit.module.ts index 205615b16..7ebafab01 100644 --- a/server/src/nest/transit/transit.module.ts +++ b/server/src/nest/transit/transit.module.ts @@ -6,6 +6,7 @@ import { TransitMcp } from './transit.mcp'; import { DaysModule } from '../days/days.module'; import { ReservationsModule } from '../reservations/reservations.module'; import { AuthModule } from '../auth/auth.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** * Transit domain (#1065) — the Transitous/MOTIS proxy. TransitMcp carries the @@ -14,7 +15,7 @@ import { AuthModule } from '../auth/auth.module'; */ @Module({ // DaysModule + ReservationsModule: TransitMcp's create_transit_journey injects both. - imports: [RateLimitModule, DaysModule, ReservationsModule, AuthModule], + imports: [McpSharedModule, RateLimitModule, DaysModule, ReservationsModule, AuthModule], controllers: [TransitController], providers: [TransitService, TransitMcp], exports: [TransitService], diff --git a/server/src/nest/trips/trips.mcp.ts b/server/src/nest/trips/trips.mcp.ts index cac913881..a2d6537e0 100644 --- a/server/src/nest/trips/trips.mcp.ts +++ b/server/src/nest/trips/trips.mcp.ts @@ -4,13 +4,14 @@ import { TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, ok, } from '@trek/nest-mcp'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; import { CalendarService } from '../calendar/calendar.service'; import { TripMembersService } from '../trip-members/trip-members.service'; import { TripReadModelService } from '../trip-read-model/trip-read-model.service'; import { ADDON_IDS } from '../../addons'; -import { safeBroadcast, MAX_MCP_TRIP_DAYS, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { MAX_MCP_TRIP_DAYS, noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { canRead, canReadTrips, canDeleteTrips } from '../../mcp/scopes'; import { TripsService, NotFoundError, ValidationError } from './trips.service'; import { TodoService } from '../todo/todo.service'; @@ -72,6 +73,7 @@ export class TripsMcp { private readonly members: TripMembersService, private readonly readModel: TripReadModelService, private readonly addons: AddonsService, + private readonly guards: McpToolGuardsService, ) {} // --- TRIPS --- @@ -140,7 +142,7 @@ export class TripsMcp { ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.trips.canAccessTrip(tripId, ctx.userId)) return noAccess(); - if (!hasTripPermission('trip_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.guards.hasTripPermission('trip_edit', tripId, ctx.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) @@ -154,7 +156,7 @@ export class TripsMcp { // update() re-anchors the budget before the trip row moves off the old // currency (#1543) and then runs the legacy updateTrip core. const { updatedTrip } = await this.trips.update(tripId, ctx.userId, { title, description, start_date, end_date, currency, is_archived, cover_image, date_shift_mode }, 'user'); - safeBroadcast(tripId, 'trip:updated', { trip: updatedTrip }); + this.guards.safeBroadcast(tripId, 'trip:updated', { trip: updatedTrip }); return ok({ trip: updatedTrip }); } @@ -302,7 +304,7 @@ export class TripsMcp { return { content: [{ type: 'text' as const, text: 'Only the trip owner can add members.' }], isError: true }; try { const result = this.members.addMember(tripId, identifier, ownerRow.user_id, ctx.userId); - safeBroadcast(tripId, 'member:added', { member: result.member }); + this.guards.safeBroadcast(tripId, 'member:added', { member: result.member }); return ok({ member: result.member }); } catch (err) { const msg = err instanceof ValidationError || err instanceof NotFoundError ? err.message : 'Failed to add member.'; @@ -327,7 +329,7 @@ export class TripsMcp { if (!ownerRow || ownerRow.user_id !== ctx.userId) return { content: [{ type: 'text' as const, text: 'Only the trip owner can remove members.' }], isError: true }; this.members.removeMember(tripId, memberId); - safeBroadcast(tripId, 'member:removed', { userId: memberId }); + this.guards.safeBroadcast(tripId, 'member:removed', { userId: memberId }); return ok({ success: true }); } diff --git a/server/src/nest/trips/trips.module.ts b/server/src/nest/trips/trips.module.ts index f6c84fcc4..2b678c21b 100644 --- a/server/src/nest/trips/trips.module.ts +++ b/server/src/nest/trips/trips.module.ts @@ -26,11 +26,12 @@ import { AccommodationsModule } from '../accommodations/accommodations.module'; import { TripMembersModule } from '../trip-members/trip-members.module'; import { TripReadModelModule } from '../trip-read-model/trip-read-model.module'; import { AddonsModule } from '../addons/addons.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; /** Trips aggregate root (C1 — Phase 3). Uses exact strangler prefixes so it does * not capture the nested sub-domain mounts (collab, files, ...). */ @Module({ - imports: [TodoModule, PackingModule, FilesModule, ReservationsModule, DaysModule, PermissionsModule, AuditModule, BudgetModule, CollabModule, VacayModule, PlacesModule, AuthModule, AppConfigModule, UnsplashModule, RealtimeModule, PluginGuardsModule, AddonsModule, TripMembershipModule, CalendarModule, AccommodationsModule, TripMembersModule, TripReadModelModule], + imports: [McpSharedModule, TodoModule, PackingModule, FilesModule, ReservationsModule, DaysModule, PermissionsModule, AuditModule, BudgetModule, CollabModule, VacayModule, PlacesModule, AuthModule, AppConfigModule, UnsplashModule, RealtimeModule, PluginGuardsModule, AddonsModule, TripMembershipModule, CalendarModule, AccommodationsModule, TripMembersModule, TripReadModelModule], controllers: [TripsController], providers: [TripsService, TripsMcp, TripPromptsMcp, TripsRpc], // Exported for FeedsModule (ICS feeds) and PluginsModule (RPC host injection). diff --git a/server/tests/e2e/transit.e2e.test.ts b/server/tests/e2e/transit.e2e.test.ts index 2e5cc7d06..db397dce6 100644 --- a/server/tests/e2e/transit.e2e.test.ts +++ b/server/tests/e2e/transit.e2e.test.ts @@ -21,9 +21,6 @@ const { db } = vi.hoisted(() => { return { db: tmp }; }); vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: () => {} })); -// TransitMcp (in the module graph, never invoked here) reaches _shared → -// src/websocket at import time — silence the real ws module. -vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); import { TransitModule } from '../../src/nest/transit/transit.module'; import { TransitService } from '../../src/nest/transit/transit.service'; diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index abfebb25a..ba4d6434e 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -51,6 +51,7 @@ import { TripsService } from '../../src/nest/trips/trips.service'; import { VacayMcp } from '../../src/nest/vacay/vacay.mcp'; import { VacayService } from '../../src/nest/vacay/vacay.service'; import { RealtimeService } from '../../src/nest/realtime/realtime.service'; +import { McpToolGuardsService } from '../../src/nest/mcp-shared/mcp-tool-guards.service'; import { QueryHelpersService } from '../../src/nest/query-helpers/query-helpers.service'; import { JourneyMcp } from '../../src/nest/journey/journey.mcp'; import { JourneyDomainService } from '../../src/nest/journey/journey-domain.service'; @@ -90,6 +91,7 @@ export function createMcpTestRegistry(): McpRegistry { // old shape, so `membership` and `webauthn` held the wrong objects and // userCleanup/mailer/tokens were undefined. const realtimeService = new RealtimeService(); + const guards = new McpToolGuardsService(dbService, permissionsService, realtimeService); const exchangeRatesService = new ExchangeRatesService(); const budgetService = new BudgetService(dbService, permissionsService, exchangeRatesService, realtimeService); const authService = new AuthService( @@ -155,23 +157,23 @@ export function createMcpTestRegistry(): McpRegistry { new WeatherMcp(new WeatherService()), new AirportsMcp(), new AuthMcp(), - new TodoMcp(todoService, authService, addonsService), - new PackingMcp(packingService, authService, addonsService), - new BudgetMcp(budgetService, exchangeRatesService, dbService, new RuntimeEnvService(), new TripMembershipService(dbService), addonsService), - new ReservationsMcp(reservationsService, daysService, budgetService, authService, assignmentsService), - new DayNotesMcp(new DayNotesService(dbService, permissionsService, realtimeService), authService), - new DaysMcp(daysService, authService), - new AccommodationsMcp(accommodationsService, dbService, placesService, authService), - new AssignmentsMcp(assignmentsService, daysService, authService), - new CollabMcp(collabService, authService, addonsService), + new TodoMcp(todoService, authService, addonsService, guards), + new PackingMcp(packingService, authService, addonsService, guards), + new BudgetMcp(budgetService, exchangeRatesService, dbService, new RuntimeEnvService(), new TripMembershipService(dbService), addonsService, guards), + new ReservationsMcp(reservationsService, daysService, budgetService, authService, assignmentsService, guards), + new DayNotesMcp(new DayNotesService(dbService, permissionsService, realtimeService), authService, guards), + new DaysMcp(daysService, authService, guards), + new AccommodationsMcp(accommodationsService, dbService, placesService, authService, guards), + new AssignmentsMcp(assignmentsService, daysService, authService, guards), + new CollabMcp(collabService, authService, addonsService, guards), new VacayMcp(new VacayService(dbService, realtimeService, notificationsStub()), authService, addonsService), - new TripsMcp(tripsService, todoService, collabService, authService, calendarService, membersService, readModelService, addonsService), + new TripsMcp(tripsService, todoService, collabService, authService, calendarService, membersService, readModelService, addonsService, guards), new TripPromptsMcp(tripsService, readModelService, packingService, addonsService), - new ShareMcp(new ShareService(dbService, new SettingsService(dbService), permissionsService, queryHelpersService, placePhotoCache), authService), + new ShareMcp(new ShareService(dbService, new SettingsService(dbService), permissionsService, queryHelpersService, placePhotoCache), authService, guards), new MapsMcp(mapsService), - new PlacesMcp(placesService, mapsService, dbService, authService, journeyDomain, assignmentsService), + new PlacesMcp(placesService, mapsService, dbService, authService, journeyDomain, assignmentsService, guards), new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService, notificationsStub()), dbService, authService, addonsService), - new TransitMcp(new TransitService(), daysService, reservationsService, dbService, authService), + new TransitMcp(new TransitService(), daysService, reservationsService, dbService, authService, guards), new AtlasMcp(new AtlasService(dbService), addonsService, authService), new JourneyMcp(journeyDomain, new JourneyShareService(dbService, journeyDomain), addonsService, authService), new NotificationsMcp(makeNotificationsService(dbService, realtimeService), authService), diff --git a/server/tests/unit/mcp/tools-prompts.test.ts b/server/tests/unit/mcp/tools-prompts.test.ts index 4a687f5d2..fef8a184e 100644 --- a/server/tests/unit/mcp/tools-prompts.test.ts +++ b/server/tests/unit/mcp/tools-prompts.test.ts @@ -76,6 +76,7 @@ import { AuthMcp } from '../../../src/nest/auth/auth.mcp'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import { McpToolGuardsService } from '../../../src/nest/mcp-shared/mcp-tool-guards.service'; import type { AuthService } from '../../../src/nest/auth/auth.service'; import type { TripsService } from '../../../src/nest/trips/trips.service'; import type { TodoService } from '../../../src/nest/todo/todo.service'; @@ -94,6 +95,7 @@ const tripsStub = { const readModelStub = { getTripSummary: (tripId: number, viewerUserId?: number) => mockGetTripSummary(tripId, viewerUserId), } as never; +const promptGuards = new McpToolGuardsService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService()); const tripsMcp = new TripsMcp( tripsStub, { listItems: () => [] } as unknown as TodoService, @@ -103,6 +105,7 @@ const tripsMcp = new TripsMcp( undefined as never, readModelStub, addonsStub, + promptGuards, ); // The three remaining prompts moved to their domains' @McpController classes: @@ -111,7 +114,7 @@ const tripsMcp = new TripsMcp( const promptDbs = () => new DatabaseService(testDb); const authStub = { isDemoUser: () => false } as unknown as AuthService; const promptPackingService = new PackingService(promptDbs(), new PermissionsService(promptDbs()), new RealtimeService(), notificationsStub()); -const packingMcp = new PackingMcp(promptPackingService, authStub, addonsStub); +const packingMcp = new PackingMcp(promptPackingService, authStub, addonsStub, promptGuards); const budgetMcp = new BudgetMcp( new BudgetService(promptDbs(), new PermissionsService(promptDbs()), new ExchangeRatesService(), new RealtimeService()), new ExchangeRatesService(), @@ -119,6 +122,7 @@ const budgetMcp = new BudgetMcp( new RuntimeEnvService(), new TripMembershipService(promptDbs()), addonsStub, + promptGuards, ); // The packing-list / budget-overview prompts live here since the trips.bridge // fold; the summary rides the same readModelStub the trip-summary prompt uses. diff --git a/server/tests/unit/nest/mcp-shared.service.test.ts b/server/tests/unit/nest/mcp-shared.service.test.ts new file mode 100644 index 000000000..f28ce5bf5 --- /dev/null +++ b/server/tests/unit/nest/mcp-shared.service.test.ts @@ -0,0 +1,108 @@ +/** + * McpToolGuardsService — the injectable fold of the impure _shared.ts helpers. + * A real in-memory SQLite backs hasTripPermission/isAdminUser so the SQL stays + * byte-faithful to the module functions it replaces; broadcast flows through + * the vi.mock'd src/websocket, exactly like every .mcp.ts consumer expects. + */ +import { describe, it, expect, vi, beforeAll, beforeEach } from 'vitest'; + +const { testDb, dbMock, broadcastMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + return { testDb: db, dbMock: { db, closeDb: () => {}, reinitialize: () => {} }, broadcastMock: vi.fn() }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock })); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { createUser } from '../../helpers/factories'; +import { McpToolGuardsService } from '../../../src/nest/mcp-shared/mcp-tool-guards.service'; +import { McpSharedModule } from '../../../src/nest/mcp-shared/mcp-shared.module'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; + +const dbs = new DatabaseService(testDb); +const svc = new McpToolGuardsService(dbs, new PermissionsService(dbs), new RealtimeService()); + +function createTrip(ownerId: number): number { + const r = testDb.prepare("INSERT INTO trips (user_id, title) VALUES (?, 'T')").run(ownerId); + return Number(r.lastInsertRowid); +} + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + vi.clearAllMocks(); + testDb.prepare('DELETE FROM trip_members').run(); + testDb.prepare('DELETE FROM trips').run(); + testDb.prepare('DELETE FROM users').run(); +}); + +describe('hasTripPermission', () => { + it('GRD-001: false for a missing trip', () => { + const { user } = createUser(testDb); + expect(svc.hasTripPermission('trip_edit', 99999, user.id)).toBe(false); + }); + + it('GRD-002: the owner passes owner-level actions; a stranger does not', () => { + const { user: owner } = createUser(testDb); + const { user: stranger } = createUser(testDb); + const tripId = createTrip(owner.id); + expect(svc.hasTripPermission('trip_delete', tripId, owner.id)).toBe(true); + expect(svc.hasTripPermission('trip_delete', tripId, stranger.id)).toBe(false); + }); + + it('GRD-003: an unknown user falls back to the plain user role', () => { + const { user: owner } = createUser(testDb); + const tripId = createTrip(owner.id); + expect(svc.hasTripPermission('trip_delete', tripId, 424242)).toBe(false); + }); + + it('GRD-004: a global admin passes regardless of membership', () => { + const { user: owner } = createUser(testDb); + const { user: admin } = createUser(testDb); + testDb.prepare("UPDATE users SET role = 'admin' WHERE id = ?").run(admin.id); + const tripId = createTrip(owner.id); + expect(svc.hasTripPermission('trip_delete', tripId, admin.id)).toBe(true); + }); +}); + +describe('isAdminUser', () => { + it('GRD-010: reflects the users.role column and is false for unknown ids', () => { + const { user } = createUser(testDb); + expect(svc.isAdminUser(user.id)).toBe(false); + testDb.prepare("UPDATE users SET role = 'admin' WHERE id = ?").run(user.id); + expect(svc.isAdminUser(user.id)).toBe(true); + expect(svc.isAdminUser(424242)).toBe(false); + }); +}); + +describe('safeBroadcast', () => { + it('GRD-020: stamps _source: mcp and flows through the mocked websocket seam', () => { + svc.safeBroadcast(7, 'todo:created', { item: { id: 1 } }); + expect(broadcastMock).toHaveBeenCalledWith(7, 'todo:created', { item: { id: 1 }, _source: 'mcp' }); + }); + + it('GRD-021: swallows broadcast failures so a tool result is never lost to a ws error', () => { + broadcastMock.mockImplementationOnce(() => { throw new Error('ws down'); }); + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => svc.safeBroadcast(7, 'todo:created', {})).not.toThrow(); + expect(errSpy).toHaveBeenCalled(); + errSpy.mockRestore(); + }); +}); + +describe('McpSharedModule', () => { + it('GRD-030: provides and exports the guards over the permissions domain', () => { + expect(Reflect.getMetadata('providers', McpSharedModule)).toEqual([McpToolGuardsService]); + expect(Reflect.getMetadata('exports', McpSharedModule)).toEqual([McpToolGuardsService]); + }); +}); diff --git a/server/tests/unit/nest/permissions.service.test.ts b/server/tests/unit/nest/permissions.service.test.ts index 665b8a717..97342743b 100644 --- a/server/tests/unit/nest/permissions.service.test.ts +++ b/server/tests/unit/nest/permissions.service.test.ts @@ -43,7 +43,6 @@ import { runMigrations } from '../../../src/db/migrations'; import { logError } from '../../../src/nest/audit/audit-log.logger'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService, PERMISSION_ACTIONS } from '../../../src/nest/permissions/permissions.service'; -import { checkPermission as bridgeCheckPermission } from '../../../src/nest/permissions/permissions.bridge'; import { invalidatePermissionsCache as invalidateSharedCache } from '../../../src/nest/permissions/permissions-cache'; const svc = new PermissionsService(new DatabaseService(testDb)); @@ -238,19 +237,21 @@ describe('stored overrides + cache', () => { }); }); -// ── permissions.bridge delegation ───────────────────────────────────────────── +// ── module-scoped cache across instances ────────────────────────────────────── -describe('permissions.bridge delegation', () => { - it('PERM-SVC-017: checkPermission delegates through the bridge instance', () => { - expect(bridgeCheckPermission('trip_create', 'user', null, 42, false)).toBe(true); - expect(bridgeCheckPermission('trip_delete', 'user', 10, 20, true)).toBe(false); +describe('module-scoped permissions cache', () => { + const secondInstance = new PermissionsService(new DatabaseService(testDb)); + + it('PERM-SVC-017: checkPermission agrees across independently built instances', () => { + expect(secondInstance.checkPermission('trip_create', 'user', null, 42, false)).toBe(true); + expect(secondInstance.checkPermission('trip_delete', 'user', 10, 20, true)).toBe(false); }); - it('PERM-SVC-020: the cache is module-scoped — shared by the DI and bridge instances', () => { - // Save through the DI instance; the bridge instance sees it immediately + it('PERM-SVC-020: the cache is module-scoped — shared by every service instance', () => { + // Save through the DI instance; a second instance sees it immediately // (checkPermission for a plain member flips with the stored level). svc.savePermissions({ trip_edit: 'trip_member' }); - expect(bridgeCheckPermission('trip_edit', 'user', 10, 20, true)).toBe(true); + expect(secondInstance.checkPermission('trip_edit', 'user', 10, 20, true)).toBe(true); // Raw SQL write, then invalidate through permissions-cache — the plain // function backup.impl.ts calls after a restore. Both service instances // must serve the fresh value afterwards. @@ -258,6 +259,6 @@ describe('permissions.bridge delegation', () => { expect(svc.getPermissionLevel('trip_edit')).toBe('trip_member'); // still cached invalidateSharedCache(); expect(svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); - expect(bridgeCheckPermission('trip_edit', 'user', 10, 20, true)).toBe(false); + expect(secondInstance.checkPermission('trip_edit', 'user', 10, 20, true)).toBe(false); }); });