diff --git a/server/src/nest/plugins/host/plugin-host-deps.factory.ts b/server/src/nest/plugins/host/plugin-host-deps.factory.ts index 18c0da3ab..ad9b998cd 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -38,6 +38,7 @@ import { FilesService } from '../../files/files.service'; import { CollabService } from '../../collab/collab.service'; import { VacayService } from '../../vacay/vacay.service'; import { PluginRpcHost, ForbiddenResource, BadParams } from './rpc-host'; +import { PluginRpcRegistryService } from './rpc-kit/registry.service'; import { appendAudit } from './plugin-audit'; import { getPluginDataDb, budgetFor } from './plugin-host-state'; @@ -167,6 +168,10 @@ export class PluginHostDepsFactory { private readonly notifications: NotificationsService, private readonly membership: TripMembershipService, private readonly journey: JourneyDomainService, + // Appended last and OPTIONAL so the hand-wired 26-arg construction in + // plugin-host-deps.factory.test.ts keeps compiling. Handed to every host it + // builds, which is how decorated *.rpc.ts handlers reach the dispatch map. + private readonly rpcRegistry?: PluginRpcRegistryService, ) {} /** @@ -858,18 +863,6 @@ export class PluginHostDepsFactory { listCategories: () => this.categories.list() as unknown[], tripMembers: (tripId) => this.db.prepare('SELECT u.id, u.username, u.display_name, u.avatar FROM trip_members tm JOIN users u ON u.id = tm.user_id WHERE tm.trip_id = ?').all(tripId) as unknown[], - // --- Tags (the acting user's own; ownership re-checked before a write). --- - listTagsForUser: (userId) => this.tags.list(userId) as unknown[], - createTagForUser: (userId, name, color) => this.tags.create(userId, name, color), - updateTagForUser: (userId, tagId, name, color) => { - if (!this.tags.getByIdAndUser(tagId, userId)) throw new ForbiddenResource(`no tag ${tagId} for this user`); - return this.tags.update(tagId, name, color); - }, - deleteTagForUser: (userId, tagId) => { - if (!this.tags.getByIdAndUser(tagId, userId)) throw new ForbiddenResource(`no tag ${tagId} for this user`); - this.tags.remove(tagId); - return { deleted: true }; - }, // --- Todos (core, trip-scoped; the app's 'packing_edit' permission). --- canEditTodos: (tripId, userId) => this.canEditTripAs('packing_edit', tripId, userId), listTodos: (tripId) => this.todos.listItems(String(tripId)) as unknown[], @@ -939,6 +932,6 @@ export class PluginHostDepsFactory { .run(id, entityType, entityId, key); return { deleted: res.changes > 0 }; }, - }); + }, this.rpcRegistry); } } diff --git a/server/src/nest/plugins/host/rpc-host.ts b/server/src/nest/plugins/host/rpc-host.ts index 3db712c32..cf7d2db60 100644 --- a/server/src/nest/plugins/host/rpc-host.ts +++ b/server/src/nest/plugins/host/rpc-host.ts @@ -21,6 +21,14 @@ import { import type { PluginDataDb } from './plugin-data.service'; import { stripEmoji } from '../text-sanitize'; import { auditResource, isAuditable } from './plugin-audit'; +import { BadParams, ForbiddenResource } from './rpc-errors'; +import { asArgs, asPayload, asTxOps, num, str } from './rpc-params'; +import type { PluginRpcRegistry } from './rpc-kit/registry'; + +// Both used to be declared here. They now live in rpc-errors.ts so decorated +// *.rpc.ts handlers can throw them without importing the router, and are re-exported +// so every existing importer keeps working unchanged. +export { BadParams, ForbiddenResource }; /** * The per-plugin capability router (#plugins, M1) — the ENFORCEMENT POINT. @@ -33,9 +41,6 @@ import { auditResource, isAuditable } from './plugin-audit'; * Runs in the HOST (parent) process. */ -/** Thrown by a handler when the acting user may not touch the requested resource. */ -export class ForbiddenResource extends Error {} - interface CoreDb { prepare(sql: string): { all(...args: unknown[]): unknown[]; @@ -240,10 +245,6 @@ export interface HostDeps { listCategories(): unknown[]; tripMembers(tripId: number): unknown[]; // --- Tags (the acting user's own; no trip) --- - listTagsForUser(userId: number): unknown[]; - createTagForUser(userId: number, name: string, color: string | undefined): unknown; - updateTagForUser(userId: number, tagId: number, name: string | undefined, color: string | undefined): unknown; - deleteTagForUser(userId: number, tagId: number): unknown; // --- Todos (core, trip-scoped; the 'packing_edit' permission, like the REST path) --- canEditTodos(tripId: number, userId: number): boolean; listTodos(tripId: number): unknown[]; @@ -261,17 +262,6 @@ export interface HostDeps { type Handler = (params: Record, actingUserId: number | undefined) => unknown; -const num = (v: unknown, name: string): number => { - const n = typeof v === 'string' ? Number(v) : v; - if (typeof n !== 'number' || !Number.isFinite(n)) throw new BadParams(`${name} must be a number`); - return n; -}; -const str = (v: unknown, name: string): string => { - if (typeof v !== 'string') throw new BadParams(`${name} must be a string`); - return v; -}; -export class BadParams extends Error {} - // Mirrors the STRING_LIMITS the places REST controller enforces (the @trek/shared // schema doesn't), so the plugin write path rejects the same oversized fields. const PLACE_STR_LIMITS: Record = { name: 200, description: 2000, address: 500, notes: 2000 }; @@ -289,6 +279,11 @@ export class PluginRpcHost { private readonly pluginId: string, granted: ReadonlySet, private readonly deps: HostDeps, + // Additive and OPTIONAL, the same convention plugin-runtime.service.ts already + // uses for its own late-added params. Every existing + // `new PluginRpcHost(id, granted, deps)` site keeps compiling untouched, which is + // what makes the decorator rollout a per-domain move instead of a big-bang one. + registry?: PluginRpcRegistry, ) { const has = (p: string) => granted.has(p); @@ -951,30 +946,7 @@ export class PluginRpcHost { // Global, read-only reference list — carries no tenant data. this.methods.set('categories.list', () => deps.listCategories()); } - if (has('db:read:tags')) { - // The acting user's own tags (not trip-scoped) — refuse a userless context. - this.methods.set('tags.list', (_p, uid) => { - if (uid === undefined) throw new ForbiddenResource('tag reads require an authenticated user context'); - return deps.listTagsForUser(uid); - }); - } - if (has('db:write:tags')) { - this.methods.set('tags.create', (p, uid) => { - if (uid === undefined) throw new ForbiddenResource('tag writes require an authenticated user context'); - const input = asPayload(p.input); - if (typeof input.name !== 'string' || input.name.trim() === '') throw new BadParams('tag name is required'); - return deps.createTagForUser(uid, input.name, typeof input.color === 'string' ? input.color : undefined); - }); - this.methods.set('tags.update', (p, uid) => { - if (uid === undefined) throw new ForbiddenResource('tag writes require an authenticated user context'); - const input = asPayload(p.input); - return deps.updateTagForUser(uid, num(p.tagId, 'tagId'), typeof input.name === 'string' ? input.name : undefined, typeof input.color === 'string' ? input.color : undefined); - }); - this.methods.set('tags.delete', (p, uid) => { - if (uid === undefined) throw new ForbiddenResource('tag writes require an authenticated user context'); - return deps.deleteTagForUser(uid, num(p.tagId, 'tagId')); - }); - } + // tags.* now lives in src/nest/tags/tags.rpc.ts, bound through the registry below. if (has('db:read:todos')) { this.methods.set('todos.list', (p, uid) => this.tripRead(p, uid, () => deps.listTodos(num(p.tripId, 'tripId')))); } @@ -1155,6 +1127,21 @@ export class PluginRpcHost { if (uid === undefined) return { value: undefined }; return { value: deps.getUserSetting(this.pluginId, uid, str(p.key, 'key')) }; }); + + // LAST on purpose: a method still owned by one of the has() blocks above then + // collides loudly in bindInto instead of being silently overridden, which is the + // failure mode a half-finished domain move would otherwise have. + // + // `deps.data` is already the lazy per-access getter the factory supplies, so + // passing it through a getter here preserves the disable/re-enable semantics + // exactly and the kit never needs to know about plugin-host-state. + registry?.bindInto(this.methods, granted, (actingUserId) => ({ + pluginId, + actingUserId, + get data() { + return deps.data; + }, + })); } /** @@ -1242,7 +1229,14 @@ export class PluginRpcHost { } async dispatch(req: RpcRequest, actingUserId?: number): Promise { - const params = (req.params ?? {}) as Record; + const raw = (req.params ?? {}) as Record; + // The supervisor resolves the acting user from `_inv` BEFORE dispatch and passes + // the request on untouched, so `_inv` is visible to every handler today and is in + // effect a reserved param name. Strip it here: no handler and no audit reads + // anything but named fields, and dropping it keeps a future strict schema parse + // from rejecting calls every shipped plugin makes. + const { _inv, ...stripped } = raw; + const params = '_inv' in raw ? stripped : raw; const res = await this.handle(req, params, actingUserId); // Audit the core-data / broadcast surface (incl. denials) at the boundary. if (this.deps.audit && isAuditable(req.method)) { @@ -1301,23 +1295,6 @@ export class PluginRpcHost { } } -function asArgs(v: unknown): unknown[] { - if (v == null) return []; - if (Array.isArray(v)) return v; - throw new BadParams('args must be an array'); -} -/** Coerce a db.tx `ops` param into a validated {sql, args}[] before it reaches the - * data db — each op must carry a string sql and, if present, an array of args. */ -function asTxOps(v: unknown): Array<{ sql: string; args?: unknown[] }> { - if (!Array.isArray(v)) throw new BadParams('ops must be an array of { sql, args }'); - return v.map((op) => { - const o = (op ?? {}) as Record; - return { sql: str(o.sql, 'ops[].sql'), args: asArgs(o.args) }; - }); -} -function asPayload(v: unknown): Record { - return v && typeof v === 'object' ? (v as Record) : { value: v }; -} /** * The reservation body is passthrough by contract, but a malformed `endpoints` * array would otherwise fail DEEP in the service (NOT-NULL mid-transaction) or be diff --git a/server/src/nest/plugins/plugins.module.ts b/server/src/nest/plugins/plugins.module.ts index a9318669a..e0deaa63f 100644 --- a/server/src/nest/plugins/plugins.module.ts +++ b/server/src/nest/plugins/plugins.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { DiscoveryModule } from '@nestjs/core'; import { JourneyDomainModule } from '../journey/journey-domain.module'; import { PluginsController } from './plugins.controller'; import { PluginsFeedController } from './plugins-feed.controller'; @@ -26,6 +27,8 @@ import { PluginsService } from './plugins.service'; import { PluginRuntimeService } from './plugin-runtime.service'; import { PluginRegistryService } from './registry/registry.service'; import { PluginHostDepsFactory } from './host/plugin-host-deps.factory'; +import { PluginRpcRegistryService } from './host/rpc-kit/registry.service'; +import { PluginGuards } from './host/plugin-guards.service'; import { TagsModule } from '../tags/tags.module'; import { CategoriesModule } from '../categories/categories.module'; import { BudgetModule } from '../budget/budget.module'; @@ -58,11 +61,15 @@ import { TripMembershipModule } from '../trip-membership/trip-membership.module' @Module({ // The DI-native domain services the plugin host wiring injects // (PluginHostDepsFactory); DatabaseModule is @Global, so not listed. - imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule, FilesModule, CollabModule, VacayModule, TripsModule, PermissionsModule, AuditModule, AddonsModule, PlacesModule, CollectionsModule, AtlasModule, NotificationsModule, TripMembershipModule, JourneyDomainModule], + // DiscoveryModule is what lets PluginRpcRegistryService find the @PluginController + // providers at boot. + imports: [DiscoveryModule, TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule, FilesModule, CollabModule, VacayModule, TripsModule, PermissionsModule, AuditModule, AddonsModule, PlacesModule, CollectionsModule, AtlasModule, NotificationsModule, TripMembershipModule, JourneyDomainModule], controllers: [PluginsController, PluginsFeedController, PluginsProxyController, PluginFrameController, PlaceDetailsController, TripWarningsController, ViewContributionsController, TripCardContributionsController, PluginPhotosController, PluginCalendarController, MapMarkersController, MapLayersController, PluginRoutesController, DayScheduleController, DayTintsController, PdfSectionsController, AtlasLayersController, JournalEntryRowsController, PluginUserSettingsController, PluginOAuthController, PluginActivityController], - providers: [PluginsService, PluginRuntimeService, PluginRegistryService, PluginOAuthService, PluginHostDepsFactory], - // Exported so the admin addon-toggle handler can cascade-disable plugins whose - // required addon was just turned off (#plugins dependencies). - exports: [PluginRuntimeService], + providers: [PluginsService, PluginRuntimeService, PluginRegistryService, PluginOAuthService, PluginHostDepsFactory, PluginRpcRegistryService, PluginGuards], + // PluginRuntimeService is exported so the admin addon-toggle handler can + // cascade-disable plugins whose required addon was just turned off (#plugins + // dependencies). PluginGuards is exported so a domain module can inject it into + // its own .rpc.ts without importing the plugin host. + exports: [PluginRuntimeService, PluginGuards], }) export class PluginsModule {} diff --git a/server/src/nest/tags/tags.module.ts b/server/src/nest/tags/tags.module.ts index aa3d685ae..2d5fa8d0c 100644 --- a/server/src/nest/tags/tags.module.ts +++ b/server/src/nest/tags/tags.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { TagsController } from './tags.controller'; import { TagsMcp } from './tags.mcp'; +import { TagsRpc } from './tags.rpc'; import { TagsService } from './tags.service'; import { AuthModule } from '../auth/auth.module'; @@ -8,7 +9,10 @@ import { AuthModule } from '../auth/auth.module'; @Module({ imports: [AuthModule], controllers: [TagsController], - providers: [TagsService, TagsMcp], + // TagsRpc must stay in providers: the plugin RPC registry discovers marked + // PROVIDERS only, and a missing entry here would leave tags.* answering + // PERMISSION_DENIED with no other symptom. + providers: [TagsService, TagsMcp, TagsRpc], // For in-container consumers (PluginHostDepsFactory). exports: [TagsService], }) diff --git a/server/src/nest/tags/tags.rpc.ts b/server/src/nest/tags/tags.rpc.ts new file mode 100644 index 000000000..6a0e53fab --- /dev/null +++ b/server/src/nest/tags/tags.rpc.ts @@ -0,0 +1,71 @@ +import { PluginController, PluginMethod } from '../plugins/host/rpc-kit/decorators'; +import { BadParams, ForbiddenResource } from '../plugins/host/rpc-errors'; +import { asPayload, num } from '../plugins/host/rpc-params'; +import type { PluginRpcContext } from '../plugins/host/rpc-kit/types'; +import { TagsService } from './tags.service'; + +/** + * The tags surface a plugin may reach (#plugins). Tags are the acting user's own and + * are not trip-scoped, so there is no membership gate here; what every handler does + * need is a HOST-bound acting user, and a write additionally re-checks that the tag + * belongs to that user. + * + * Moved verbatim out of the router and the deps factory. The messages differ by one + * word between the read and the writes ("tag reads" vs "tag writes"), which is why + * the read throws inline instead of going through PluginGuards.requireActor. + */ +@PluginController() +export class TagsRpc { + constructor(private readonly tags: TagsService) {} + + @PluginMethod('tags.list', { permission: 'db:read:tags' }) + list(_params: Record, ctx: PluginRpcContext): unknown[] { + if (ctx.actingUserId === undefined) { + throw new ForbiddenResource('tag reads require an authenticated user context'); + } + return this.tags.list(ctx.actingUserId) as unknown[]; + } + + @PluginMethod('tags.create', { permission: 'db:write:tags' }) + create(params: Record, ctx: PluginRpcContext): unknown { + if (ctx.actingUserId === undefined) { + throw new ForbiddenResource('tag writes require an authenticated user context'); + } + const input = asPayload(params.input); + if (typeof input.name !== 'string' || input.name.trim() === '') throw new BadParams('tag name is required'); + return this.tags.create(ctx.actingUserId, input.name, typeof input.color === 'string' ? input.color : undefined); + } + + @PluginMethod('tags.update', { permission: 'db:write:tags' }) + update(params: Record, ctx: PluginRpcContext): unknown { + if (ctx.actingUserId === undefined) { + throw new ForbiddenResource('tag writes require an authenticated user context'); + } + const input = asPayload(params.input); + const tagId = num(params.tagId, 'tagId'); + this.requireOwnTag(tagId, ctx.actingUserId); + return this.tags.update( + tagId, + typeof input.name === 'string' ? input.name : undefined, + typeof input.color === 'string' ? input.color : undefined, + ); + } + + @PluginMethod('tags.delete', { permission: 'db:write:tags' }) + delete(params: Record, ctx: PluginRpcContext): unknown { + if (ctx.actingUserId === undefined) { + throw new ForbiddenResource('tag writes require an authenticated user context'); + } + const tagId = num(params.tagId, 'tagId'); + this.requireOwnTag(tagId, ctx.actingUserId); + this.tags.remove(tagId); + return { deleted: true }; + } + + /** Ownership is re-checked per write, so a plugin cannot edit another user's tag. */ + private requireOwnTag(tagId: number, userId: number): void { + if (!this.tags.getByIdAndUser(tagId, userId)) { + throw new ForbiddenResource(`no tag ${tagId} for this user`); + } + } +} diff --git a/server/tests/helpers/rpc-host-deps.ts b/server/tests/helpers/rpc-host-deps.ts new file mode 100644 index 000000000..1fa2c67bb --- /dev/null +++ b/server/tests/helpers/rpc-host-deps.ts @@ -0,0 +1,184 @@ +import { vi } from 'vitest'; +import type { HostDeps } from '../../src/nest/plugins/host/rpc-host'; +import { TagsRpc } from '../../src/nest/tags/tags.rpc'; +import type { TagsService } from '../../src/nest/tags/tags.service'; + +/** + * The stubbed HostDeps every plugin router test builds on. It used to live inside + * rpc-host.test.ts; it moved here so the coverage ledger test can construct the same + * host without duplicating 150 lines of stubs. + * + * The fixture encodes one access story throughout: user 42 may access and edit trip 1, + * and nothing else. Tests rely on that, so widen it only with care. + */ +export function makeDeps(): HostDeps { + return { + data: { + query: vi.fn(() => [{ n: 1 }]), + exec: vi.fn(() => ({ changes: 1 })), + migrate: vi.fn(() => ({ applied: true })), + close: vi.fn(), + } as unknown as HostDeps['data'], + db: { + prepare: vi.fn((sql: string) => ({ + all: () => [{ id: 7, name: 'Place' }], + get: () => + sql.includes('FROM trips') + ? { id: 1, title: 'Japan', start_date: '2027-01-01' } + : { id: 3, username: 'ada', display_name: 'Ada', avatar: null }, + })), + }, + // trip 1 is accessible to user 42; everything else is not + canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1 } : undefined)), + // user 42 may see user 3 (they share a trip); nobody else + canSeeUser: vi.fn((actingUserId: number, targetUserId: number) => actingUserId === 42 && targetUserId === 3), + broadcastToTrip: vi.fn(), + broadcastToUser: vi.fn(), + // Costs (budget) — addon on; user 42 may edit trip 1's costs. + budgetAddonEnabled: vi.fn(() => true), + canEditCosts: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + listPackingItems: vi.fn((tripId: number, _userId: number) => [{ id: 1, trip_id: tripId, name: 'Socks' }]), + listTripFiles: vi.fn((tripId: number) => [{ id: 2, trip_id: tripId, filename: 'visa.pdf' }]), + getTripFileContent: vi.fn((tripId: number, fileId: number) => ({ name: 'visa.pdf', mimetype: 'application/pdf', size: 3, content_base64: 'aGk=', _t: tripId, _f: fileId })), + listCollabNotes: vi.fn((tripId: number) => [{ id: 1, trip_id: tripId, title: 'Note' }]), + listCollabPolls: vi.fn((tripId: number) => [{ id: 2, trip_id: tripId, question: 'Q?' }]), + listCollabMessages: vi.fn((tripId: number, before: number | undefined) => [{ id: 3, trip_id: tripId, text: 'hi', _before: before ?? null }]), + journalEntriesForUser: vi.fn((uid: number, journeyId: number) => [{ id: 10, journey_id: journeyId, author_id: uid }]), + atlasBucketForUser: vi.fn((uid: number) => [{ id: 5, user_id: uid, name: 'Kyoto' }]), + canCreateTrip: vi.fn((userId: number) => userId === 42), + createTripForUser: vi.fn((userId: number, input: unknown) => ({ id: 99, user_id: userId, ...(input as object) })), + getRates: vi.fn(async (base: string) => ({ [base]: 1, USD: 1.08 })), + listCostsForTrip: vi.fn((tripId: number) => [{ id: 5, trip_id: tripId, name: 'Hotel', total_price: 100 }]), + listCostsForUser: vi.fn(() => [ + { id: 5, trip_id: 1, name: 'Hotel' }, + { id: 6, trip_id: 2, name: 'Food' }, + ]), + createCost: vi.fn((tripId: number, input: unknown) => ({ id: 9, trip_id: tripId, ...(input as object) })), + updateCost: vi.fn((tripId: number, itemId: number, input: unknown) => ({ id: itemId, trip_id: tripId, ...(input as object) })), + deleteCost: vi.fn(() => ({ deleted: true })), + // Planner writes — user 42 may edit trip 1 only (mirrors canAccessTrip). + canEditPlaces: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + createPlace: vi.fn((tripId: number, input: unknown) => ({ id: 10, trip_id: tripId, ...(input as object) })), + updatePlace: vi.fn((tripId: number, placeId: number, input: unknown) => ({ id: placeId, trip_id: tripId, ...(input as object) })), + deletePlace: vi.fn(() => ({ deleted: true })), + canEditDays: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + createDay: vi.fn((tripId: number, input: unknown) => ({ id: 20, trip_id: tripId, ...(input as object) })), + updateDay: vi.fn((tripId: number, dayId: number, input: unknown) => ({ id: dayId, trip_id: tripId, ...(input as object) })), + deleteDay: vi.fn(() => ({ deleted: true })), + assignPlaceToDay: vi.fn((tripId: number, dayId: number, placeId: number, notes: string | null) => ({ id: 30, day_id: dayId, place_id: placeId, notes })), + unassignPlace: vi.fn(() => ({ deleted: true })), + canEditTrip: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + updateTrip: vi.fn((tripId: number, _userId: number, input: unknown) => ({ id: tripId, ...(input as object) })), + // Cross-trip reads + reservations (bookings) — user 42 may edit trip 1 only. + listTripsForUser: vi.fn(() => [{ id: 1, title: 'Japan' }, { id: 2, title: 'Peru' }]), + listReservationsForUser: vi.fn(() => [{ id: 5, trip_id: 1, title: 'Hotel' }, { id: 6, trip_id: 2, title: 'Flight' }]), + canEditReservations: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + createReservation: vi.fn((tripId: number, input: unknown) => ({ id: 40, trip_id: tripId, ...(input as object) })), + updateReservation: vi.fn((tripId: number, reservationId: number, input: unknown) => ({ id: reservationId, trip_id: tripId, ...(input as object) })), + deleteReservation: vi.fn(() => ({ deleted: true })), + // Trip-scoped hydrated reads + accommodations (lodging blocks, day_edit-gated). + listTripDays: vi.fn((tripId: number) => [{ id: 3, trip_id: tripId, day_number: 1, assignments: [], notes_items: [] }]), + listTripReservations: vi.fn((tripId: number) => [{ id: 5, trip_id: tripId, title: 'Hotel', endpoints: [], day_positions: null }]), + listTripAccommodations: vi.fn((tripId: number) => [{ id: 11, trip_id: tripId, place_name: 'Ryokan' }]), + createAccommodation: vi.fn((tripId: number, input: unknown) => ({ id: 60, trip_id: tripId, ...(input as object) })), + updateAccommodation: vi.fn((tripId: number, accommodationId: number, input: unknown) => ({ id: accommodationId, trip_id: tripId, ...(input as object) })), + deleteAccommodation: vi.fn(() => ({ deleted: true })), + // User-scoped addon reads + day notes. + listJournalsForUser: vi.fn(() => [{ id: 1, title: 'Japan 2027' }]), + atlasVisitedForUser: vi.fn(() => ({ countries: [{ country_code: 'JP' }], regions: [] })), + vacayForUser: vi.fn(() => ({ plan: { id: 1 }, entries: [] })), + listCollectionsForUser: vi.fn(() => ({ collections: [{ id: 1, name: 'Tokyo eats' }] })), + getCollectionForUser: vi.fn((_userId: number, id: number) => ({ id, name: 'Tokyo eats', places: [] })), + listDayNotes: vi.fn((tripId: number, dayId: number) => [{ id: 1, day_id: dayId, trip_id: tripId, text: 'note' }]), + createDayNote: vi.fn((_tripId: number, dayId: number, input: unknown) => ({ id: 50, day_id: dayId, ...(input as object) })), + updateDayNote: vi.fn((_tripId: number, dayId: number, noteId: number, input: unknown) => ({ id: noteId, day_id: dayId, ...(input as object) })), + deleteDayNote: vi.fn(() => ({ deleted: true })), + canEditPacking: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + createPackingItem: vi.fn((tripId: number, input: unknown) => ({ id: 70, trip_id: tripId, ...(input as object) })), + updatePackingItem: vi.fn((tripId: number, itemId: number, input: unknown) => ({ id: itemId, trip_id: tripId, ...(input as object) })), + deletePackingItem: vi.fn(() => ({ deleted: true })), + listPackingBags: vi.fn(() => [{ id: 80, name: 'Backpack' }]), + createPackingBag: vi.fn((tripId: number, input: unknown) => ({ id: 80, trip_id: tripId, ...(input as object) })), + updatePackingBag: vi.fn((_tripId: number, bagId: number, input: unknown) => ({ id: bagId, ...(input as object) })), + deletePackingBag: vi.fn(() => ({ deleted: true })), + setPackingBagMembers: vi.fn((_tripId: number, bagId: number, userIds: number[]) => ({ bagId, members: userIds })), + getWeather: vi.fn(() => ({ temp: 20 })), + listCategories: vi.fn(() => [{ id: 1, name: 'Food' }]), + tripMembers: vi.fn(() => [{ id: 5, username: 'ada' }, { id: 6, username: 'bob' }]), + canUploadFiles: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + canEditFiles: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + canDeleteFiles: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + createTripFile: vi.fn((tripId: number, input: unknown, uid: number) => ({ id: 130, trip_id: tripId, uploaded_by: uid, ...(input as object) })), + createTripFileLink: vi.fn(() => [{ file_id: 130 }]), + updateTripFile: vi.fn((_tripId: number, fileId: number, input: unknown) => ({ id: fileId, ...(input as object) })), + softDeleteTripFile: vi.fn(() => ({ deleted: true })), + canEditCollab: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + createCollabNote: vi.fn((tripId: number, input: unknown, uid: number) => ({ id: 140, trip_id: tripId, created_by: uid, ...(input as object) })), + createCollabPoll: vi.fn((tripId: number, input: unknown) => ({ id: 141, trip_id: tripId, ...(input as object) })), + voteCollabPoll: vi.fn((_tripId: number, pollId: number) => ({ id: pollId, votes: 1 })), + createCollabMessage: vi.fn((tripId: number, text: string) => ({ id: 142, trip_id: tripId, text })), + canManageMembers: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + addTripMember: vi.fn((tripId: number, targetUserId: number, invitedBy: number) => ({ joined: true, tripId, targetUserId, invitedBy })), + removeTripMember: vi.fn((tripId: number, targetUserId: number) => ({ removed: true, tripId, targetUserId })), + canAccessTripForNotify: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + sendPluginNotification: vi.fn(async (pluginId: string, input: unknown) => ({ sent: true, pluginId, ...(input as object) })), + aiConfigured: vi.fn((userId: number) => userId === 42), + aiComplete: vi.fn(async (_uid: number, prompt: string) => ({ text: `echo:${prompt}` })), + aiExtract: vi.fn(async () => ({ results: [{ ok: true }] })), + getUserSetting: vi.fn((pluginId: string, userId: number, key: string) => (userId === 42 && key === 'apiKey' ? `secret-of-${pluginId}` : undefined)), + getOAuthToken: vi.fn(async (_pluginId: string, userId: number) => (userId === 42 ? 'access-token-xyz' : null)), + schedulerSet: vi.fn((name: string, dueAt: number, everyMs: number | undefined) => ({ scheduled: true, name, dueAt, everyMs })), + schedulerCancel: vi.fn((name: string) => ({ cancelled: name !== 'ghost' })), + createCollectionForUser: vi.fn((uid: number, input: unknown) => ({ id: 100, owner_id: uid, ...(input as object) })), + updateCollectionForUser: vi.fn((_uid: number, id: number, input: unknown) => ({ id, ...(input as object) })), + saveCollectionPlace: vi.fn((uid: number, input: unknown) => ({ id: 101, saved_by: uid, ...(input as object) })), + copyCollectionToTrip: vi.fn(() => ({ copied: 2, skipped: [] })), + deleteCollectionPlace: vi.fn(() => ({ deleted: true })), + markCountryVisited: vi.fn(() => ({ visited: true })), + unmarkCountryVisited: vi.fn(() => ({ visited: false })), + markRegionVisited: vi.fn(() => ({ visited: true })), + unmarkRegionVisited: vi.fn(() => ({ visited: false })), + createBucketItem: vi.fn((uid: number, input: unknown) => ({ id: 110, user_id: uid, ...(input as object) })), + deleteBucketItem: vi.fn(() => ({ deleted: true })), + vacayToggleEntry: vi.fn(() => ({ action: 'added' })), + vacayToggleCompanyHoliday: vi.fn(() => ({ action: 'added' })), + createJournalEntry: vi.fn((uid: number, journeyId: number, input: unknown) => ({ id: 120, journey_id: journeyId, created_by: uid, ...(input as object) })), + updateJournalEntry: vi.fn((_uid: number, entryId: number, input: unknown) => ({ id: entryId, ...(input as object) })), + deleteJournalEntry: vi.fn(() => ({ deleted: true })), + createJournal: vi.fn((uid: number, input: unknown) => ({ id: 130, user_id: uid, ...(input as object) })), + deleteJournal: vi.fn(() => ({ deleted: true })), + canEditTodos: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), + listTodos: vi.fn(() => [{ id: 1, name: 'Pack' }]), + createTodo: vi.fn((tripId: number, input: unknown) => ({ id: 90, trip_id: tripId, ...(input as object) })), + updateTodo: vi.fn((_tripId: number, todoId: number, input: unknown) => ({ id: todoId, ...(input as object) })), + deleteTodo: vi.fn(() => ({ deleted: true })), + // Metadata — trip 1 and place 7 resolve to trip 1 (accessible to 42); else undefined. + metaEntityTrip: vi.fn((entityType: string, entityId: number) => + (entityType === 'trip' && entityId === 1) || (entityType === 'place' && entityId === 7) || (entityType === 'day' && entityId === 3) + || (entityType === 'reservation' && entityId === 40) || (entityType === 'accommodation' && entityId === 11) ? 1 : undefined), + metaGet: vi.fn(() => ({ hello: 'world' })), + metaSet: vi.fn((_et: string, _eid: number, key: string, value: unknown) => ({ key, value })), + metaList: vi.fn(() => ({ a: 1 })), + metaDelete: vi.fn(() => ({ deleted: true })), + }; +} + +/** + * Every @PluginController() class that owns wire methods, in the order the container + * would discover them. It grows by one entry per rollout PR, and the coverage ledger + * test (tests/unit/plugins/rpc-coverage.test.ts) is what turns a forgotten entry into + * a red build instead of a silent PERMISSION_DENIED in production. + * + * The services handed in are stubs: the ledger only binds the handlers, it never runs + * them. Behaviour is asserted in each domain's own .rpc.test.ts. + */ +export function allRpcControllers(): object[] { + const tags = { + list: vi.fn(() => []), + getByIdAndUser: vi.fn(() => undefined), + create: vi.fn(() => ({})), + update: vi.fn(() => ({})), + remove: vi.fn(), + } as unknown as TagsService; + return [new TagsRpc(tags)]; +} diff --git a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts index e2161d2f2..8c81fb3fb 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -800,15 +800,8 @@ describe('host-deps factory — Wave 1 wiring (weather/categories/tags/todos/ros expect((await call(host('db:read:categories'), 'categories.list', {}, undefined)).ok).toBe(true) }) - it('tags: list/create/update/delete are the acting user\'s; a missing tag is RESOURCE_FORBIDDEN', async () => { - const h = host('db:read:tags', 'db:write:tags') - expect((await call(h, 'tags.list', {}, 5)).ok).toBe(true) - expect((await call(h, 'tags.create', { input: { name: 'x' } }, 5)).ok).toBe(true) - expect((await call(h, 'tags.update', { tagId: 1, input: { name: 'y' } }, 5)).ok).toBe(true) - expect((await call(h, 'tags.delete', { tagId: 1 }, 5)).ok).toBe(true) - expect(((await call(h, 'tags.update', { tagId: 404, input: {} }, 5)) as { error: { code: string } }).error.code).toBe('RESOURCE_FORBIDDEN') - expect(((await call(h, 'tags.delete', { tagId: 404 }, 5)) as { error: { code: string } }).error.code).toBe('RESOURCE_FORBIDDEN') - }) + // tags.* left this factory with the decorator migration. The same cases now run in + // tests/unit/tags/tags.rpc.test.ts against TagsRpc. it('trips.members returns the roster', async () => { const r = await call(host('db:read:trips'), 'trips.members', { tripId: 1 }, 5) diff --git a/server/tests/unit/plugins/rpc-coverage.test.ts b/server/tests/unit/plugins/rpc-coverage.test.ts new file mode 100644 index 000000000..382b3650a --- /dev/null +++ b/server/tests/unit/plugins/rpc-coverage.test.ts @@ -0,0 +1,64 @@ +/** + * The coverage ledger: the anchor for the whole decorator rollout. + * + * Every wire method is owned exactly once, either by the legacy router blocks in + * rpc-host.ts or by a decorated *.rpc.ts class in the registry. Both sets are derived + * by construction, so no list is maintained by hand and the test cannot drift. + * + * It fails the moment a migration PR moves a method and forgets to delete the legacy + * registration, deletes one without adding it, or forgets to list a new *.rpc.ts class + * in allRpcControllers(). + */ +import { describe, it, expect } from 'vitest'; +import { PluginRpcHost } from '../../../src/nest/plugins/host/rpc-host'; +import { createTestPluginRegistry } from '../../../src/nest/plugins/host/rpc-kit/testing'; +import { + KNOWN_METHODS, + KNOWN_PERMISSIONS, + UNCONDITIONAL_METHODS, +} from '../../../src/nest/plugins/protocol/envelope'; +import { allRpcControllers, makeDeps } from '../../helpers/rpc-host-deps'; + +/** Reads the router's private dispatch map. That map IS the thing under test. */ +const boundMethods = (host: PluginRpcHost): Set => + new Set((host as unknown as { methods: Map }).methods.keys()); + +describe('plugin RPC coverage ledger', () => { + const registry = createTestPluginRegistry(allRpcControllers()); + // Every permission granted, so the router registers everything it can. + const host = new PluginRpcHost('probe', new Set(KNOWN_PERMISSIONS), makeDeps(), registry); + const all = boundMethods(host); + const fromRegistry = registry.methodNames(); + const legacy = new Set([...all].filter((m) => !fromRegistry.has(m))); + + it('RPCLEDGER-001 no method is owned by both the registry and the legacy router', () => { + expect([...fromRegistry].filter((m) => legacy.has(m))).toEqual([]); + }); + + it('RPCLEDGER-002 every registered method is one the wire protocol knows', () => { + const vocabulary = new Set([...KNOWN_METHODS, ...UNCONDITIONAL_METHODS]); + expect([...all].filter((m) => !vocabulary.has(m))).toEqual([]); + }); + + it('RPCLEDGER-003 no method in the vocabulary is orphaned', () => { + const vocabulary = new Set([...KNOWN_METHODS, ...UNCONDITIONAL_METHODS]); + expect([...vocabulary].filter((m) => !all.has(m))).toEqual([]); + }); + + it('RPCLEDGER-004 the two owner sets partition the vocabulary exactly', () => { + expect(all).toEqual(new Set([...KNOWN_METHODS, ...UNCONDITIONAL_METHODS])); + expect(fromRegistry.size + legacy.size).toBe(all.size); + }); + + it('RPCLEDGER-005 the migration ledger reads as expected for this PR', () => { + // Update these two numbers in every rollout PR. They make the diff state the size + // of the move out loud, which is the number a reviewer wants to see. + const total = KNOWN_METHODS.length + UNCONDITIONAL_METHODS.length; + expect(fromRegistry.size).toBe(4); // tags.list / create / update / delete + expect(legacy.size).toBe(total - 4); + }); + + it('RPCLEDGER-006 the migrated methods are exactly the ones this PR claims', () => { + expect([...fromRegistry].sort()).toEqual(['tags.create', 'tags.delete', 'tags.list', 'tags.update']); + }); +}); diff --git a/server/tests/unit/plugins/rpc-host-inv-strip.test.ts b/server/tests/unit/plugins/rpc-host-inv-strip.test.ts new file mode 100644 index 000000000..b73011533 --- /dev/null +++ b/server/tests/unit/plugins/rpc-host-inv-strip.test.ts @@ -0,0 +1,70 @@ +/** + * The supervisor tags every host-bound request with `_inv`, resolves the acting user + * from it BEFORE dispatch, and then passes the request on untouched. That made `_inv` + * visible to every handler and, in effect, a reserved param name. + * + * dispatch() now strips it. These tests prove the strip is invisible: the same result, + * the same audit row, and the acting user still resolved from the host side. + */ +import { describe, it, expect, vi } from 'vitest'; +import { PluginRpcHost } from '../../../src/nest/plugins/host/rpc-host'; +import type { RpcRequest, RpcResponse } from '../../../src/nest/plugins/protocol/envelope'; +import { makeDeps } from '../../helpers/rpc-host-deps'; + +const req = (method: string, params: Record): RpcRequest => ({ k: 'req', id: 'x', method, params }); + +describe('dispatch strips the supervisor _inv marker', () => { + it('RPCINV-001 the handler never sees _inv', async () => { + const deps = makeDeps(); + const host = new PluginRpcHost('p', new Set(['db:own']), deps); + await host.dispatch(req('db.query', { sql: 'SELECT 1', args: [], _inv: 'req-7' })); + expect(deps.data.query).toHaveBeenCalledWith('SELECT 1', []); + }); + + it('RPCINV-002 the result is identical with and without the marker', async () => { + const host = new PluginRpcHost('p', new Set(['db:own']), makeDeps()); + const withMarker = (await host.dispatch(req('db.query', { sql: 'SELECT 1', _inv: 'req-7' }))) as RpcResponse; + const without = (await host.dispatch(req('db.query', { sql: 'SELECT 1' }))) as RpcResponse; + expect(withMarker.ok).toBe(true); + expect(withMarker.result).toEqual(without.result); + }); + + it('RPCINV-003 the audit row is unchanged, because auditResource reads named fields only', async () => { + const deps = makeDeps(); + const audit = vi.fn(); + const host = new PluginRpcHost('p', new Set(['db:read:trips']), { ...deps, audit }); + await host.dispatch(req('trips.getById', { tripId: 1, _inv: 'req-7' }), 42); + expect(audit).toHaveBeenCalledWith({ + pluginId: 'p', + actingUserId: 42, + method: 'trips.getById', + resource: 'trip:1', + code: 'ok', + }); + }); + + it('RPCINV-004 the acting user still comes from the host, not from the params', async () => { + const deps = makeDeps(); + const host = new PluginRpcHost('p', new Set(['db:read:trips']), deps); + // No acting user bound: the trip read is refused even though _inv is present. + const res = await host.dispatch(req('trips.getById', { tripId: 1, _inv: 'req-7' })); + expect(res.ok).toBe(false); + expect((res as { error: { code: string } }).error.code).toBe('RESOURCE_FORBIDDEN'); + }); + + it('RPCINV-005 params without the marker are passed through untouched', async () => { + const deps = makeDeps(); + const host = new PluginRpcHost('p', new Set(['db:own']), deps); + await host.dispatch(req('db.exec', { sql: 'INSERT INTO t VALUES (1)', args: [1] })); + expect(deps.data.exec).toHaveBeenCalledWith('INSERT INTO t VALUES (1)', [1]); + }); + + it('RPCINV-006 a handler reading the whole payload object no longer finds _inv in it', async () => { + const deps = makeDeps(); + const host = new PluginRpcHost('p', new Set(['db:write:packing']), deps); + // packing.setBagMembers is the one handler that reads the params object as a + // whole (asPayload(p).userIds) rather than a named field off it. + await host.dispatch(req('packing.setBagMembers', { tripId: 1, bagId: 80, userIds: [3], _inv: 'req-7' }), 42); + expect(deps.setPackingBagMembers).toHaveBeenCalledWith(1, 80, [3]); + }); +}); diff --git a/server/tests/unit/plugins/rpc-host.test.ts b/server/tests/unit/plugins/rpc-host.test.ts index 344695e97..39c05694b 100644 --- a/server/tests/unit/plugins/rpc-host.test.ts +++ b/server/tests/unit/plugins/rpc-host.test.ts @@ -10,162 +10,8 @@ import Database from 'better-sqlite3'; import { createTables } from '../../../src/db/schema'; import { PluginRpcHost, type HostDeps } from '../../../src/nest/plugins/host/rpc-host'; import type { RpcRequest, RpcResponse, RpcError } from '../../../src/nest/plugins/protocol/envelope'; +import { makeDeps } from '../../helpers/rpc-host-deps'; -function makeDeps(): HostDeps { - return { - data: { - query: vi.fn(() => [{ n: 1 }]), - exec: vi.fn(() => ({ changes: 1 })), - migrate: vi.fn(() => ({ applied: true })), - close: vi.fn(), - } as unknown as HostDeps['data'], - db: { - prepare: vi.fn((sql: string) => ({ - all: () => [{ id: 7, name: 'Place' }], - get: () => - sql.includes('FROM trips') - ? { id: 1, title: 'Japan', start_date: '2027-01-01' } - : { id: 3, username: 'ada', display_name: 'Ada', avatar: null }, - })), - }, - // trip 1 is accessible to user 42; everything else is not - canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1 } : undefined)), - // user 42 may see user 3 (they share a trip); nobody else - canSeeUser: vi.fn((actingUserId: number, targetUserId: number) => actingUserId === 42 && targetUserId === 3), - broadcastToTrip: vi.fn(), - broadcastToUser: vi.fn(), - // Costs (budget) — addon on; user 42 may edit trip 1's costs. - budgetAddonEnabled: vi.fn(() => true), - canEditCosts: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - listPackingItems: vi.fn((tripId: number, _userId: number) => [{ id: 1, trip_id: tripId, name: 'Socks' }]), - listTripFiles: vi.fn((tripId: number) => [{ id: 2, trip_id: tripId, filename: 'visa.pdf' }]), - getTripFileContent: vi.fn((tripId: number, fileId: number) => ({ name: 'visa.pdf', mimetype: 'application/pdf', size: 3, content_base64: 'aGk=', _t: tripId, _f: fileId })), - listCollabNotes: vi.fn((tripId: number) => [{ id: 1, trip_id: tripId, title: 'Note' }]), - listCollabPolls: vi.fn((tripId: number) => [{ id: 2, trip_id: tripId, question: 'Q?' }]), - listCollabMessages: vi.fn((tripId: number, before: number | undefined) => [{ id: 3, trip_id: tripId, text: 'hi', _before: before ?? null }]), - journalEntriesForUser: vi.fn((uid: number, journeyId: number) => [{ id: 10, journey_id: journeyId, author_id: uid }]), - atlasBucketForUser: vi.fn((uid: number) => [{ id: 5, user_id: uid, name: 'Kyoto' }]), - canCreateTrip: vi.fn((userId: number) => userId === 42), - createTripForUser: vi.fn((userId: number, input: unknown) => ({ id: 99, user_id: userId, ...(input as object) })), - getRates: vi.fn(async (base: string) => ({ [base]: 1, USD: 1.08 })), - listCostsForTrip: vi.fn((tripId: number) => [{ id: 5, trip_id: tripId, name: 'Hotel', total_price: 100 }]), - listCostsForUser: vi.fn(() => [ - { id: 5, trip_id: 1, name: 'Hotel' }, - { id: 6, trip_id: 2, name: 'Food' }, - ]), - createCost: vi.fn((tripId: number, input: unknown) => ({ id: 9, trip_id: tripId, ...(input as object) })), - updateCost: vi.fn((tripId: number, itemId: number, input: unknown) => ({ id: itemId, trip_id: tripId, ...(input as object) })), - deleteCost: vi.fn(() => ({ deleted: true })), - // Planner writes — user 42 may edit trip 1 only (mirrors canAccessTrip). - canEditPlaces: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - createPlace: vi.fn((tripId: number, input: unknown) => ({ id: 10, trip_id: tripId, ...(input as object) })), - updatePlace: vi.fn((tripId: number, placeId: number, input: unknown) => ({ id: placeId, trip_id: tripId, ...(input as object) })), - deletePlace: vi.fn(() => ({ deleted: true })), - canEditDays: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - createDay: vi.fn((tripId: number, input: unknown) => ({ id: 20, trip_id: tripId, ...(input as object) })), - updateDay: vi.fn((tripId: number, dayId: number, input: unknown) => ({ id: dayId, trip_id: tripId, ...(input as object) })), - deleteDay: vi.fn(() => ({ deleted: true })), - assignPlaceToDay: vi.fn((tripId: number, dayId: number, placeId: number, notes: string | null) => ({ id: 30, day_id: dayId, place_id: placeId, notes })), - unassignPlace: vi.fn(() => ({ deleted: true })), - canEditTrip: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - updateTrip: vi.fn((tripId: number, _userId: number, input: unknown) => ({ id: tripId, ...(input as object) })), - // Cross-trip reads + reservations (bookings) — user 42 may edit trip 1 only. - listTripsForUser: vi.fn(() => [{ id: 1, title: 'Japan' }, { id: 2, title: 'Peru' }]), - listReservationsForUser: vi.fn(() => [{ id: 5, trip_id: 1, title: 'Hotel' }, { id: 6, trip_id: 2, title: 'Flight' }]), - canEditReservations: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - createReservation: vi.fn((tripId: number, input: unknown) => ({ id: 40, trip_id: tripId, ...(input as object) })), - updateReservation: vi.fn((tripId: number, reservationId: number, input: unknown) => ({ id: reservationId, trip_id: tripId, ...(input as object) })), - deleteReservation: vi.fn(() => ({ deleted: true })), - // Trip-scoped hydrated reads + accommodations (lodging blocks, day_edit-gated). - listTripDays: vi.fn((tripId: number) => [{ id: 3, trip_id: tripId, day_number: 1, assignments: [], notes_items: [] }]), - listTripReservations: vi.fn((tripId: number) => [{ id: 5, trip_id: tripId, title: 'Hotel', endpoints: [], day_positions: null }]), - listTripAccommodations: vi.fn((tripId: number) => [{ id: 11, trip_id: tripId, place_name: 'Ryokan' }]), - createAccommodation: vi.fn((tripId: number, input: unknown) => ({ id: 60, trip_id: tripId, ...(input as object) })), - updateAccommodation: vi.fn((tripId: number, accommodationId: number, input: unknown) => ({ id: accommodationId, trip_id: tripId, ...(input as object) })), - deleteAccommodation: vi.fn(() => ({ deleted: true })), - // User-scoped addon reads + day notes. - listJournalsForUser: vi.fn(() => [{ id: 1, title: 'Japan 2027' }]), - atlasVisitedForUser: vi.fn(() => ({ countries: [{ country_code: 'JP' }], regions: [] })), - vacayForUser: vi.fn(() => ({ plan: { id: 1 }, entries: [] })), - listCollectionsForUser: vi.fn(() => ({ collections: [{ id: 1, name: 'Tokyo eats' }] })), - getCollectionForUser: vi.fn((_userId: number, id: number) => ({ id, name: 'Tokyo eats', places: [] })), - listDayNotes: vi.fn((tripId: number, dayId: number) => [{ id: 1, day_id: dayId, trip_id: tripId, text: 'note' }]), - createDayNote: vi.fn((_tripId: number, dayId: number, input: unknown) => ({ id: 50, day_id: dayId, ...(input as object) })), - updateDayNote: vi.fn((_tripId: number, dayId: number, noteId: number, input: unknown) => ({ id: noteId, day_id: dayId, ...(input as object) })), - deleteDayNote: vi.fn(() => ({ deleted: true })), - canEditPacking: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - createPackingItem: vi.fn((tripId: number, input: unknown) => ({ id: 70, trip_id: tripId, ...(input as object) })), - updatePackingItem: vi.fn((tripId: number, itemId: number, input: unknown) => ({ id: itemId, trip_id: tripId, ...(input as object) })), - deletePackingItem: vi.fn(() => ({ deleted: true })), - listPackingBags: vi.fn(() => [{ id: 80, name: 'Backpack' }]), - createPackingBag: vi.fn((tripId: number, input: unknown) => ({ id: 80, trip_id: tripId, ...(input as object) })), - updatePackingBag: vi.fn((_tripId: number, bagId: number, input: unknown) => ({ id: bagId, ...(input as object) })), - deletePackingBag: vi.fn(() => ({ deleted: true })), - setPackingBagMembers: vi.fn((_tripId: number, bagId: number, userIds: number[]) => ({ bagId, members: userIds })), - getWeather: vi.fn(() => ({ temp: 20 })), - listCategories: vi.fn(() => [{ id: 1, name: 'Food' }]), - tripMembers: vi.fn(() => [{ id: 5, username: 'ada' }, { id: 6, username: 'bob' }]), - listTagsForUser: vi.fn((uid: number) => [{ id: 1, user_id: uid, name: 'work' }]), - createTagForUser: vi.fn((uid: number, name: string) => ({ id: 9, user_id: uid, name })), - updateTagForUser: vi.fn((_uid: number, tagId: number, name?: string) => ({ id: tagId, name })), - deleteTagForUser: vi.fn(() => ({ deleted: true })), - canUploadFiles: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - canEditFiles: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - canDeleteFiles: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - createTripFile: vi.fn((tripId: number, input: unknown, uid: number) => ({ id: 130, trip_id: tripId, uploaded_by: uid, ...(input as object) })), - createTripFileLink: vi.fn(() => [{ file_id: 130 }]), - updateTripFile: vi.fn((_tripId: number, fileId: number, input: unknown) => ({ id: fileId, ...(input as object) })), - softDeleteTripFile: vi.fn(() => ({ deleted: true })), - canEditCollab: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - createCollabNote: vi.fn((tripId: number, input: unknown, uid: number) => ({ id: 140, trip_id: tripId, created_by: uid, ...(input as object) })), - createCollabPoll: vi.fn((tripId: number, input: unknown) => ({ id: 141, trip_id: tripId, ...(input as object) })), - voteCollabPoll: vi.fn((_tripId: number, pollId: number) => ({ id: pollId, votes: 1 })), - createCollabMessage: vi.fn((tripId: number, text: string) => ({ id: 142, trip_id: tripId, text })), - canManageMembers: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - addTripMember: vi.fn((tripId: number, targetUserId: number, invitedBy: number) => ({ joined: true, tripId, targetUserId, invitedBy })), - removeTripMember: vi.fn((tripId: number, targetUserId: number) => ({ removed: true, tripId, targetUserId })), - canAccessTripForNotify: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - sendPluginNotification: vi.fn(async (pluginId: string, input: unknown) => ({ sent: true, pluginId, ...(input as object) })), - aiConfigured: vi.fn((userId: number) => userId === 42), - aiComplete: vi.fn(async (_uid: number, prompt: string) => ({ text: `echo:${prompt}` })), - aiExtract: vi.fn(async () => ({ results: [{ ok: true }] })), - getUserSetting: vi.fn((pluginId: string, userId: number, key: string) => (userId === 42 && key === 'apiKey' ? `secret-of-${pluginId}` : undefined)), - getOAuthToken: vi.fn(async (_pluginId: string, userId: number) => (userId === 42 ? 'access-token-xyz' : null)), - schedulerSet: vi.fn((name: string, dueAt: number, everyMs: number | undefined) => ({ scheduled: true, name, dueAt, everyMs })), - schedulerCancel: vi.fn((name: string) => ({ cancelled: name !== 'ghost' })), - createCollectionForUser: vi.fn((uid: number, input: unknown) => ({ id: 100, owner_id: uid, ...(input as object) })), - updateCollectionForUser: vi.fn((_uid: number, id: number, input: unknown) => ({ id, ...(input as object) })), - saveCollectionPlace: vi.fn((uid: number, input: unknown) => ({ id: 101, saved_by: uid, ...(input as object) })), - copyCollectionToTrip: vi.fn(() => ({ copied: 2, skipped: [] })), - deleteCollectionPlace: vi.fn(() => ({ deleted: true })), - markCountryVisited: vi.fn(() => ({ visited: true })), - unmarkCountryVisited: vi.fn(() => ({ visited: false })), - markRegionVisited: vi.fn(() => ({ visited: true })), - unmarkRegionVisited: vi.fn(() => ({ visited: false })), - createBucketItem: vi.fn((uid: number, input: unknown) => ({ id: 110, user_id: uid, ...(input as object) })), - deleteBucketItem: vi.fn(() => ({ deleted: true })), - vacayToggleEntry: vi.fn(() => ({ action: 'added' })), - vacayToggleCompanyHoliday: vi.fn(() => ({ action: 'added' })), - createJournalEntry: vi.fn((uid: number, journeyId: number, input: unknown) => ({ id: 120, journey_id: journeyId, created_by: uid, ...(input as object) })), - updateJournalEntry: vi.fn((_uid: number, entryId: number, input: unknown) => ({ id: entryId, ...(input as object) })), - deleteJournalEntry: vi.fn(() => ({ deleted: true })), - createJournal: vi.fn((uid: number, input: unknown) => ({ id: 130, user_id: uid, ...(input as object) })), - deleteJournal: vi.fn(() => ({ deleted: true })), - canEditTodos: vi.fn((tripId: number, userId: number) => tripId === 1 && userId === 42), - listTodos: vi.fn(() => [{ id: 1, name: 'Pack' }]), - createTodo: vi.fn((tripId: number, input: unknown) => ({ id: 90, trip_id: tripId, ...(input as object) })), - updateTodo: vi.fn((_tripId: number, todoId: number, input: unknown) => ({ id: todoId, ...(input as object) })), - deleteTodo: vi.fn(() => ({ deleted: true })), - // Metadata — trip 1 and place 7 resolve to trip 1 (accessible to 42); else undefined. - metaEntityTrip: vi.fn((entityType: string, entityId: number) => - (entityType === 'trip' && entityId === 1) || (entityType === 'place' && entityId === 7) || (entityType === 'day' && entityId === 3) - || (entityType === 'reservation' && entityId === 40) || (entityType === 'accommodation' && entityId === 11) ? 1 : undefined), - metaGet: vi.fn(() => ({ hello: 'world' })), - metaSet: vi.fn((_et: string, _eid: number, key: string, value: unknown) => ({ key, value })), - metaList: vi.fn(() => ({ a: 1 })), - metaDelete: vi.fn(() => ({ deleted: true })), - }; -} const req = (method: string, params: Record = {}): RpcRequest => ({ k: 'req', id: 'x', method, params }); const ok = (r: RpcResponse | RpcError): r is RpcResponse => r.ok === true; @@ -545,14 +391,7 @@ describe('PluginRpcHost — capability enforcement', () => { expect(ok(await c.dispatch(req('categories.list', {}), undefined))).toBe(true); }); - it('tags are the acting user\'s own; a userless context + empty name are refused', async () => { - const host = new PluginRpcHost('p', new Set(['db:read:tags', 'db:write:tags']), deps); - expect(ok(await host.dispatch(req('tags.list'), 42))).toBe(true); - expect(deps.listTagsForUser).toHaveBeenCalledWith(42); - expect(ok(await host.dispatch(req('tags.create', { input: { name: 'work' } }), 42))).toBe(true); - expect((await host.dispatch(req('tags.list'), undefined)).ok).toBe(false); - expect((await host.dispatch(req('tags.create', { input: { name: '' } }), 42)).ok).toBe(false); - }); + // The tags cases moved to tests/unit/tags/tags.rpc.test.ts with the handlers. it('trips.members + todos.list are trip-membership-gated', async () => { const host = new PluginRpcHost('p', new Set(['db:read:trips', 'db:read:todos']), deps); diff --git a/server/tests/unit/tags/tags.rpc.test.ts b/server/tests/unit/tags/tags.rpc.test.ts new file mode 100644 index 000000000..477893850 --- /dev/null +++ b/server/tests/unit/tags/tags.rpc.test.ts @@ -0,0 +1,186 @@ +/** + * The tags plugin surface, after it moved from the router's has() blocks onto + * @PluginMethod. The dispatch-level cases are the ones that used to live in + * rpc-host.test.ts: same requests, same expected codes. Only the construction line + * and the stub identity changed, which is the diff a reviewer should be reading. + * + * The rest are the per-handler cases the router-level test never had room for, in + * particular the ownership re-check on the two writes. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { PluginRpcHost } from '../../../src/nest/plugins/host/rpc-host'; +import { createTestPluginRegistry } from '../../../src/nest/plugins/host/rpc-kit/testing'; +import { BadParams, ForbiddenResource } from '../../../src/nest/plugins/host/rpc-errors'; +import { TagsRpc } from '../../../src/nest/tags/tags.rpc'; +import { TagsModule } from '../../../src/nest/tags/tags.module'; +import type { TagsService } from '../../../src/nest/tags/tags.service'; +import type { PluginRpcContext } from '../../../src/nest/plugins/host/rpc-kit/types'; +import type { RpcRequest, RpcError } from '../../../src/nest/plugins/protocol/envelope'; +import { makeDeps } from '../../helpers/rpc-host-deps'; + +const req = (method: string, params: Record = {}): RpcRequest => ({ k: 'req', id: 'x', method, params }); +const ctx = (actingUserId: number | undefined): PluginRpcContext => ({ + pluginId: 'p', + actingUserId, + data: {} as PluginRpcContext['data'], +}); + +/** Tag 1 belongs to user 42, nothing else does. */ +function makeTags() { + return { + list: vi.fn((userId: number) => [{ id: 1, user_id: userId, name: 'work' }]), + getByIdAndUser: vi.fn((id: string | number, userId: number) => + Number(id) === 1 && userId === 42 ? { id: 1, user_id: 42, name: 'work' } : undefined, + ), + create: vi.fn((userId: number, name: string, color?: string) => ({ id: 9, user_id: userId, name, color })), + update: vi.fn((id: string | number, name?: string, color?: string) => ({ id, name, color })), + remove: vi.fn(), + } as unknown as TagsService & Record>; +} + +describe('TagsRpc through the router', () => { + let tags: ReturnType; + let host: PluginRpcHost; + + beforeEach(() => { + tags = makeTags(); + host = new PluginRpcHost( + 'p', + new Set(['db:read:tags', 'db:write:tags']), + makeDeps(), + createTestPluginRegistry([new TagsRpc(tags)]), + ); + }); + + it('TAGS-RPC-001 tags are the acting user\'s own; a userless context + empty name are refused', async () => { + expect((await host.dispatch(req('tags.list'), 42)).ok).toBe(true); + expect(tags.list).toHaveBeenCalledWith(42); + expect((await host.dispatch(req('tags.create', { input: { name: 'work' } }), 42)).ok).toBe(true); + expect((await host.dispatch(req('tags.list'), undefined)).ok).toBe(false); + expect((await host.dispatch(req('tags.create', { input: { name: '' } }), 42)).ok).toBe(false); + }); + + it('TAGS-RPC-002 a userless read is RESOURCE_FORBIDDEN and never touches the service', async () => { + const res = (await host.dispatch(req('tags.list'), undefined)) as RpcError; + expect(res.error.code).toBe('RESOURCE_FORBIDDEN'); + expect(res.error.message).toBe('tag reads require an authenticated user context'); + expect(tags.list).not.toHaveBeenCalled(); + }); + + it('TAGS-RPC-003 a blank name is BAD_PARAMS, not a refusal', async () => { + const res = (await host.dispatch(req('tags.create', { input: { name: ' ' } }), 42)) as RpcError; + expect(res.error.code).toBe('BAD_PARAMS'); + expect(res.error.message).toBe('tag name is required'); + expect(tags.create).not.toHaveBeenCalled(); + }); + + it('TAGS-RPC-004 editing another user\'s tag is RESOURCE_FORBIDDEN', async () => { + const res = (await host.dispatch(req('tags.update', { tagId: 2, input: { name: 'x' } }), 42)) as RpcError; + expect(res.error.code).toBe('RESOURCE_FORBIDDEN'); + expect(res.error.message).toBe('no tag 2 for this user'); + expect(tags.update).not.toHaveBeenCalled(); + }); + + it('TAGS-RPC-005 db:read:tags alone does not unlock a write', async () => { + const readOnly = new PluginRpcHost( + 'p', + new Set(['db:read:tags']), + makeDeps(), + createTestPluginRegistry([new TagsRpc(tags)]), + ); + const res = (await readOnly.dispatch(req('tags.create', { input: { name: 'work' } }), 42)) as RpcError; + expect(res.error.code).toBe('PERMISSION_DENIED'); + expect(tags.create).not.toHaveBeenCalled(); + }); + + it('TAGS-RPC-006 db:write:tags alone does not unlock the read', async () => { + const writeOnly = new PluginRpcHost( + 'p', + new Set(['db:write:tags']), + makeDeps(), + createTestPluginRegistry([new TagsRpc(tags)]), + ); + expect(((await writeOnly.dispatch(req('tags.list'), 42)) as RpcError).error.code).toBe('PERMISSION_DENIED'); + }); + + it('TAGS-RPC-007 with no tags grant at all, every method is PERMISSION_DENIED', async () => { + const none = new PluginRpcHost('p', new Set([]), makeDeps(), createTestPluginRegistry([new TagsRpc(tags)])); + for (const method of ['tags.list', 'tags.create', 'tags.update', 'tags.delete']) { + expect(((await none.dispatch(req(method, { tagId: 1 }), 42)) as RpcError).error.code).toBe('PERMISSION_DENIED'); + } + }); +}); + +describe('TagsRpc is reachable at all', () => { + it('TAGS-RPC-017 the class is listed in its module providers', () => { + // The one failure the compiler cannot see: a decorated class that no module + // registers is invisible to discovery, and every tags.* call then comes back + // PERMISSION_DENIED with no other symptom. Cheapest possible guard against it. + const providers = Reflect.getMetadata('providers', TagsModule) as unknown[] | undefined; + expect(providers).toContain(TagsRpc); + }); +}); + +describe('TagsRpc handlers', () => { + let tags: ReturnType; + let rpc: TagsRpc; + + beforeEach(() => { + tags = makeTags(); + rpc = new TagsRpc(tags); + }); + + it('TAGS-RPC-008 list returns the acting user\'s tags', () => { + expect(rpc.list({}, ctx(42))).toEqual([{ id: 1, user_id: 42, name: 'work' }]); + }); + + it('TAGS-RPC-009 create passes name and colour through', () => { + expect(rpc.create({ input: { name: 'work', color: '#abc' } }, ctx(42))).toEqual({ + id: 9, + user_id: 42, + name: 'work', + color: '#abc', + }); + }); + + it('TAGS-RPC-010 a non-string colour is dropped rather than passed on', () => { + rpc.create({ input: { name: 'work', color: 123 } }, ctx(42)); + expect(tags.create).toHaveBeenCalledWith(42, 'work', undefined); + }); + + it('TAGS-RPC-011 a non-object input is wrapped, so the name check still refuses it', () => { + expect(() => rpc.create({ input: 'work' }, ctx(42))).toThrow(new BadParams('tag name is required')); + }); + + it('TAGS-RPC-012 a userless write says "writes", not "reads"', () => { + expect(() => rpc.create({ input: { name: 'x' } }, ctx(undefined))).toThrow( + new ForbiddenResource('tag writes require an authenticated user context'), + ); + expect(() => rpc.update({ tagId: 1 }, ctx(undefined))).toThrow( + new ForbiddenResource('tag writes require an authenticated user context'), + ); + expect(() => rpc.delete({ tagId: 1 }, ctx(undefined))).toThrow( + new ForbiddenResource('tag writes require an authenticated user context'), + ); + }); + + it('TAGS-RPC-013 update only forwards the fields it was given', () => { + rpc.update({ tagId: 1, input: { name: 'renamed' } }, ctx(42)); + expect(tags.update).toHaveBeenCalledWith(1, 'renamed', undefined); + }); + + it('TAGS-RPC-014 a missing tagId is BAD_PARAMS before the ownership check runs', () => { + expect(() => rpc.update({ input: { name: 'x' } }, ctx(42))).toThrow(new BadParams('tagId must be a number')); + expect(tags.getByIdAndUser).not.toHaveBeenCalled(); + }); + + it('TAGS-RPC-015 delete re-checks ownership and reports the deletion', () => { + expect(rpc.delete({ tagId: 1 }, ctx(42))).toEqual({ deleted: true }); + expect(tags.remove).toHaveBeenCalledWith(1); + }); + + it('TAGS-RPC-016 deleting another user\'s tag never reaches remove()', () => { + expect(() => rpc.delete({ tagId: 2 }, ctx(42))).toThrow(new ForbiddenResource('no tag 2 for this user')); + expect(tags.remove).not.toHaveBeenCalled(); + }); +});