diff --git a/client/src/mobile/screens/dashboard/MDashWidgets.tsx b/client/src/mobile/screens/dashboard/MDashWidgets.tsx index 13ced7f83..1dd35ecad 100644 --- a/client/src/mobile/screens/dashboard/MDashWidgets.tsx +++ b/client/src/mobile/screens/dashboard/MDashWidgets.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useState } from 'react' +import React, { useCallback, useEffect, useRef, useState } from 'react' import { useNavigate } from 'react-router' import { ArrowRight, ArrowRightLeft, Bookmark, Calendar, ChevronDown, ChevronRight, @@ -103,20 +103,37 @@ function MCurrencyWidget(): React.ReactElement { const [amount, setAmount] = useState('100') const [rates, setRates] = useState | null>(null) + // The request outlives the widget: this is a third-party endpoint on a mobile + // screen a user can leave immediately, and both handlers below set state. Left + // unguarded it settles after unmount, React schedules an update against a gone + // tree, and under a test runner that surfaces as an unhandled rejection long + // after the case that triggered it has passed. + const inFlight = useRef(null) + const fetchRates = useCallback(() => { - fetch(`https://api.frankfurter.dev/v2/rates?base=${from}`) + inFlight.current?.abort() + const controller = new AbortController() + inFlight.current = controller + fetch(`https://api.frankfurter.dev/v2/rates?base=${from}`, { signal: controller.signal }) .then(r => r.json()) .then((d: Array<{ quote: string; rate: number }>) => { + if (controller.signal.aborted) return if (!Array.isArray(d)) { setRates(null); return } // Frankfurter omits the base's own self-rate; seed it so `from` stays selectable. const map: Record = { [from]: 1 } for (const r of d) map[r.quote] = r.rate setRates(map) }) - .catch(() => setRates(null)) + .catch(() => { + // An abort is not a failure: it means nobody is waiting for the answer. + if (!controller.signal.aborted) setRates(null) + }) }, [from]) - useEffect(() => { fetchRates() }, [fetchRates]) + useEffect(() => { + fetchRates() + return () => inFlight.current?.abort() + }, [fetchRates]) // Same one-time localStorage → settings migration the desktop widget runs, so // a phone-only user's pre-3.1.3 pair survives an upgrade too (#1311). diff --git a/client/tests/helpers/msw/handlers/external.ts b/client/tests/helpers/msw/handlers/external.ts new file mode 100644 index 000000000..183389c21 --- /dev/null +++ b/client/tests/helpers/msw/handlers/external.ts @@ -0,0 +1,19 @@ +import { http, HttpResponse } from 'msw'; + +/** + * Third-party endpoints the app calls directly, rather than through /api. + * + * Without these the request leaves the runner: MSW is started with + * `onUnhandledRequest: 'warn'`, so an unmatched call is a warning and a live + * network round trip, not a failure. That is how the mobile FX widget reached + * api.frankfurter.dev from CI and settled its promise after the test + * environment was gone, surfacing as `window is not defined` attributed to + * whichever case happened to be running. + * + * An empty rate list is the shape the widget already handles: it seeds the + * base's own self-rate and renders with nothing else selectable. + */ +export const externalHandlers = [ + http.get('https://api.frankfurter.dev/v2/rates', () => HttpResponse.json([])), + http.get('https://api.frankfurter.dev/v2/currencies', () => HttpResponse.json({})), +]; diff --git a/client/tests/helpers/msw/handlers/index.ts b/client/tests/helpers/msw/handlers/index.ts index 3459b3b26..42b0395e8 100644 --- a/client/tests/helpers/msw/handlers/index.ts +++ b/client/tests/helpers/msw/handlers/index.ts @@ -15,6 +15,7 @@ import { tagsHandlers } from './tags'; import { dayNotesHandlers } from './dayNotes'; import { adminHandlers } from './admin'; import { sharedHandlers } from './shared'; +import { externalHandlers } from './external'; export const defaultHandlers = [ ...authHandlers, @@ -34,4 +35,5 @@ export const defaultHandlers = [ ...dayNotesHandlers, ...adminHandlers, ...sharedHandlers, + ...externalHandlers, ]; diff --git a/server/tests/unit/nest/calendar.service.test.ts b/server/tests/unit/nest/calendar.service.test.ts index d27bc2274..3cab59280 100644 --- a/server/tests/unit/nest/calendar.service.test.ts +++ b/server/tests/unit/nest/calendar.service.test.ts @@ -704,18 +704,18 @@ describe('serialised output', () => { end_date: '2026-05-01', }); const day = createDay(testDb, trip.id, { date: '2026-05-01' }); - const place = createPlace(testDb, trip.id, { name: 'Tokyo Tower', address: 'Minato', lat: 35.6586, lng: 139.7454 }); + // The factories insert only the columns they model; every other column is set + // afterwards with an explicit UPDATE, the way the cases above do it. This case + // sets none, so the place has no address and the flight no reservation_time, + // and the snapshot was recorded from exactly that data: the assignment carries + // no LOCATION, and the flight is dropped from the export for having no + // placeable time. + const place = createPlace(testDb, trip.id, { name: 'Tokyo Tower', lat: 35.6586, lng: 139.7454 }); const assignment = createDayAssignment(testDb, day.id, place.id); testDb.prepare('UPDATE day_assignments SET assignment_time=?, assignment_end_time=? WHERE id=?') .run('09:00', '10:30', assignment.id); createDayNote(testDb, day.id, trip.id, { text: 'Bring the tickets', time: '08:00' }); - createReservation(testDb, trip.id, { - title: 'NH 203', - type: 'flight', - reservation_time: '2026-05-01T06:15', - confirmation_number: 'ABC123', - location: 'HND', - }); + createReservation(testDb, trip.id, { title: 'NH 203', type: 'flight' }); // DTSTAMP is the wall clock and the UID numbers are autoincrement state shared // with every case above, so both are normalised: this pins the document, not diff --git a/server/tests/unit/nest/channel-override.test.ts b/server/tests/unit/nest/channel-override.test.ts index a74ab53f7..49bc1811b 100644 --- a/server/tests/unit/nest/channel-override.test.ts +++ b/server/tests/unit/nest/channel-override.test.ts @@ -42,8 +42,9 @@ import { listChannels, getChannel, registerChannel, - type ExternalChannel, } from '../../../src/nest/notifications/channel-registry'; +// The registry consumes ExternalChannel but does not re-export it; it is declared here. +import type { ExternalChannel } from '../../../src/nest/notifications/notification-events'; const prefsDbs = new DatabaseService(testDb); const prefsSvc = new NotificationPreferencesService(prefsDbs, new MailerService(prefsDbs)); diff --git a/server/tests/unit/nest/channel-registry.test.ts b/server/tests/unit/nest/channel-registry.test.ts index 40f870a93..8d3fce04b 100644 --- a/server/tests/unit/nest/channel-registry.test.ts +++ b/server/tests/unit/nest/channel-registry.test.ts @@ -11,9 +11,9 @@ import { pluginChannelId, isPluginChannelId, __resetChannelsForTest, - type ChannelMessage, - type ExternalChannel, } from '../../../src/nest/notifications/channel-registry'; +// The channel contract lives in notification-events; channel-registry only consumes it. +import type { ChannelMessage, ExternalChannel } from '../../../src/nest/notifications/notification-events'; import { registerBuiltinChannels } from '../../../src/nest/notifications/channels/builtins'; import type { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; import type { NtfyService } from '../../../src/nest/notifications/transports/ntfy.service'; diff --git a/server/tests/unit/nest/collab.service.test.ts b/server/tests/unit/nest/collab.service.test.ts index b3b45b02f..a64990f66 100644 --- a/server/tests/unit/nest/collab.service.test.ts +++ b/server/tests/unit/nest/collab.service.test.ts @@ -43,9 +43,13 @@ vi.mock('../../../src/config', () => ({ })); vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); -// Stub checkSsrf so linkPreview tests can control SSRF behaviour +// Stub checkSsrf so linkPreview tests can control SSRF behaviour. Typed from the real +// checkSsrf rather than from the default implementation below, so the stubbed results +// stay full SsrfResult objects instead of whatever shape the first fixture happened to have. const { mockCheckSsrf, mockCreatePinnedDispatcher } = vi.hoisted(() => ({ - mockCheckSsrf: vi.fn(async () => ({ allowed: true, resolvedIp: '93.184.216.34' })), + mockCheckSsrf: vi.fn( + async () => ({ allowed: true, isPrivate: false, resolvedIp: '93.184.216.34' }), + ), mockCreatePinnedDispatcher: vi.fn(() => ({})), })); vi.mock('../../../src/utils/ssrfGuard', () => ({ @@ -73,7 +77,7 @@ beforeAll(() => { beforeEach(() => { resetTestDb(testDb); - mockCheckSsrf.mockResolvedValue({ allowed: true, resolvedIp: '93.184.216.34' }); + mockCheckSsrf.mockResolvedValue({ allowed: true, isPrivate: false, resolvedIp: '93.184.216.34' }); }); afterAll(() => { @@ -83,7 +87,7 @@ afterAll(() => { afterEach(() => { vi.unstubAllGlobals(); mockCheckSsrf.mockReset(); - mockCheckSsrf.mockResolvedValue({ allowed: true, resolvedIp: '93.184.216.34' }); + mockCheckSsrf.mockResolvedValue({ allowed: true, isPrivate: false, resolvedIp: '93.184.216.34' }); }); // ── Helpers ─────────────────────────────────────────────────────────────────── @@ -377,7 +381,8 @@ describe('linkPreview', () => { }); it('COLLAB-SVC-028: returns fallback when SSRF check blocks the URL', async () => { - mockCheckSsrf.mockResolvedValue({ allowed: false, error: 'SSRF blocked' }); + // 169.254.x is link-local, which the real guard reports as private and blocked. + mockCheckSsrf.mockResolvedValue({ allowed: false, isPrivate: true, error: 'SSRF blocked' }); const result = await svc.linkPreview('https://169.254.169.254/'); expect(result.title).toBeNull(); diff --git a/server/tests/unit/nest/days.controller.test.ts b/server/tests/unit/nest/days.controller.test.ts index 5b88a3c01..f14986828 100644 --- a/server/tests/unit/nest/days.controller.test.ts +++ b/server/tests/unit/nest/days.controller.test.ts @@ -4,6 +4,7 @@ import { DaysController } from '../../../src/nest/days/days.controller'; import { DayNotesController } from '../../../src/nest/day-notes/day-notes.controller'; import { DayNoteCreateDto, DayNoteUpdateDto } from '../../../src/nest/day-notes/day-notes.dto'; import { DayReorderError } from '../../../src/nest/days/days.service'; +import type { DayReorderDto } from '../../../src/nest/days/days.dto'; import type { DaysService } from '../../../src/nest/days/days.service'; import type { DayNotesService } from '../../../src/nest/day-notes/day-notes.service'; import type { User } from '../../../src/types'; @@ -27,6 +28,14 @@ function notesSvc(o: Partial = {}): DayNotesService { return { verifyTripAccess: vi.fn().mockReturnValue(trip), canEdit: vi.fn().mockReturnValue(true), broadcast: vi.fn(), ...o } as unknown as DayNotesService; } +// reorder() keeps the legacy raw-body guard on orderedIds, which DayReorderDto's type +// says can never fire: over HTTP the ZodValidationPipe rejects such a body first. These +// unit tests call the handler directly, so they can still reach the guard, and the +// payload is cast instead of being made to satisfy the DTO. +function rawReorderBody(body: Record): DayReorderDto { + return body as unknown as DayReorderDto; +} + describe('DaysController (parity with the legacy /api/trips/:tripId/days route)', () => { // The 404 "Trip not found" and 403 "No permission" cases moved to // trip-access.guard.test.ts with the check itself: TripAccessGuard resolves :tripId @@ -59,11 +68,11 @@ describe('DaysController (parity with the legacy /api/trips/:tripId/days route)' it('400 when orderedIds is missing', () => { - expect(thrown(() => new DaysController(daysSvc()).reorder(user, '5', {}))).toEqual({ status: 400, body: { error: 'orderedIds must be an array' } }); + expect(thrown(() => new DaysController(daysSvc()).reorder(user, '5', rawReorderBody({})))).toEqual({ status: 400, body: { error: 'orderedIds must be an array' } }); }); it('400 when orderedIds is not an array', () => { - expect(thrown(() => new DaysController(daysSvc()).reorder(user, '5', { orderedIds: 'nope' as never }))).toEqual({ status: 400, body: { error: 'orderedIds must be an array' } }); + expect(thrown(() => new DaysController(daysSvc()).reorder(user, '5', rawReorderBody({ orderedIds: 'nope' })))).toEqual({ status: 400, body: { error: 'orderedIds must be an array' } }); }); it('maps a DayReorderError to 400 with its message', () => { diff --git a/server/tests/unit/nest/days.service.test.ts b/server/tests/unit/nest/days.service.test.ts index 95ecff609..b3b100a83 100644 --- a/server/tests/unit/nest/days.service.test.ts +++ b/server/tests/unit/nest/days.service.test.ts @@ -60,6 +60,7 @@ import { getDay as bridgeGetDay, listDays as bridgeListDays } from '../../../src import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import { AccommodationsService } from '../../../src/nest/accommodations/accommodations.service'; +import type { Day } from '../../../src/types'; const svc = new DaysService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), new QueryHelpersService(new DatabaseService(testDb))); const accommodations = new AccommodationsService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService()); @@ -322,14 +323,16 @@ describe('quirk fixes', () => { it('DAY-SVC-033 — update preserves the omitted column (title-only keeps notes, notes-only keeps title)', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - let day = createDay(testDb, trip.id); - day = svc.update(day.id, day, { notes: 'Walking day' }) as never; - day = svc.update(day.id, day, { title: 'Arrival' }) as never; + // Typed as the domain Day, not as the factory's row shape: the variable is + // rebound to what update() returns, and only Day carries `notes`. + let day: Day = createDay(testDb, trip.id); + day = svc.update(day.id, day, { notes: 'Walking day' }); + day = svc.update(day.id, day, { title: 'Arrival' }); expect(day).toMatchObject({ title: 'Arrival', notes: 'Walking day' }); - day = svc.update(day.id, day, { notes: 'Museum day' }) as never; + day = svc.update(day.id, day, { notes: 'Museum day' }); expect(day).toMatchObject({ title: 'Arrival', notes: 'Museum day' }); // A present key still clears via the legacy falsy coercion. - day = svc.update(day.id, day, { notes: '' }) as never; + day = svc.update(day.id, day, { notes: '' }); expect(day.notes).toBeNull(); expect(day.title).toBe('Arrival'); }); diff --git a/server/tests/unit/nest/journey-domain.service.test.ts b/server/tests/unit/nest/journey-domain.service.test.ts index 0d6932e26..e98cebc90 100644 --- a/server/tests/unit/nest/journey-domain.service.test.ts +++ b/server/tests/unit/nest/journey-domain.service.test.ts @@ -591,7 +591,7 @@ describe('updateEntry', () => { author_id: 999999, }; - const updated = svc.updateEntry(entry.id, user.id, malicious as Parameters[2]); + const updated = svc.updateEntry(entry.id, user.id, malicious as Parameters[2]); expect(updated).not.toBeNull(); expect(updated!.title).toBe('Updated'); // legit field still applied diff --git a/server/tests/unit/nest/notifications.service.test.ts b/server/tests/unit/nest/notifications.service.test.ts index 91da8b03a..4a920ecb0 100644 --- a/server/tests/unit/nest/notifications.service.test.ts +++ b/server/tests/unit/nest/notifications.service.test.ts @@ -77,7 +77,10 @@ import { DatabaseService } from '../../../src/nest/database/database.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { NotificationsService, type NotificationPayload } from '../../../src/nest/notifications/notifications.service'; import { send as bridgeSend } from '../../../src/nest/notifications/notifications.bridge'; -import { setPluginChannelSource, type ExternalChannel } from '../../../src/nest/notifications/channel-registry'; +import { setPluginChannelSource } from '../../../src/nest/notifications/channel-registry'; +// The channel interface lives in notification-events; channel-registry only imports +// it for its own signatures and never re-exported it. +import type { ExternalChannel } from '../../../src/nest/notifications/notification-events'; import { makeNotificationsService, makeNotificationPreferencesService } from '../../helpers/notifications'; const notifications = makeNotificationsService(new DatabaseService(testDb)); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index 3159f1fc5..9d66074d1 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -89,9 +89,11 @@ import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; -import { AtlasService } from '../../../src/nest/atlas/atlas.service'; import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; import { AuthService } from '../../../src/nest/auth/auth.service'; +import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; +import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; import { OidcService } from '../../../src/nest/oidc/oidc.service'; import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; @@ -100,14 +102,18 @@ import { MailerService } from '../../../src/nest/notifications/mailer/mailer.ser const mailerStub = { sendPasswordResetEmail: vi.fn() } as unknown as MailerService; const membership = new TripMembershipService(new DatabaseService(testDb)); +// Positional, and previously shifted by one: an AtlasService sat in the membership +// slot, which AuthService no longer takes at all, so webauthn/userCleanup/mailer +// each landed one place too late and the EphemeralTokenService was missing +// entirely. Nothing failed, because no case below reaches those collaborators. const auth = new AuthService( new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), - new AtlasService(new DatabaseService(testDb)), membership, - undefined as never, // webauthn — not reached from the OIDC paths - undefined as never, // userCleanup — not reached from the OIDC paths + new WebauthnConfigService(new DatabaseService(testDb)), + new UserCleanupService(new DatabaseService(testDb)), mailerStub, + new EphemeralTokenService(), ); const svc = new OidcService(new DatabaseService(testDb), auth, membership); diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index 6429bba3f..5877d8c0d 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -81,10 +81,12 @@ import { resetTestDb } from '../../helpers/test-db'; import { createUser } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; -import { AtlasService } from '../../../src/nest/atlas/atlas.service'; +import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; import { AuthService } from '../../../src/nest/auth/auth.service'; import { PasskeyService } from '../../../src/nest/auth/passkey.service'; -import type { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; +import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; +import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; // MailerService is injected since the notifications fold — a stub instead of a @@ -97,14 +99,23 @@ const mailerStub = { sendPasswordResetEmail: vi.fn() } as unknown as MailerServi const resolveWebauthnConfigMock = vi.fn(); const webauthn = { resolve: resolveWebauthnConfigMock } as unknown as WebauthnConfigService; +// Positional, and previously shifted by one: an AtlasService sat in the +// membership slot, which AuthService no longer takes at all, so +// webauthn/userCleanup/mailer each landed one place too late and the +// EphemeralTokenService was missing entirely. Nothing failed, because the only +// thing PasskeyService asks AuthService for is generateToken, which reads the +// database and nothing else. The collaborators are real here rather than +// undefined so a future case that does reach one gets a working object; each +// takes only the DatabaseService. AuthService's own webauthn is the real +// resolver, separate from the `webauthn` switch PasskeyService is handed. const auth = new AuthService( new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), - new AtlasService(new DatabaseService(testDb)), - undefined as never, // membership — not reached from the passkey paths - undefined as never, // webauthn — PasskeyService gets its own stub below - undefined as never, // userCleanup — not reached from the passkey paths + new TripMembershipService(new DatabaseService(testDb)), + new WebauthnConfigService(new DatabaseService(testDb)), + new UserCleanupService(new DatabaseService(testDb)), mailerStub, + new EphemeralTokenService(), ); const svc = new PasskeyService(new DatabaseService(testDb), auth, webauthn); diff --git a/server/tests/unit/nest/realtime.service.test.ts b/server/tests/unit/nest/realtime.service.test.ts index 48916a340..e08b615c1 100644 --- a/server/tests/unit/nest/realtime.service.test.ts +++ b/server/tests/unit/nest/realtime.service.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { TrekWsPayload, TrekWsUserEventName } from '@trek/shared'; const { broadcast, broadcastToUser } = vi.hoisted(() => ({ broadcast: vi.fn(), @@ -10,6 +11,19 @@ import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; const svc = new RealtimeService(); +/** + * These cases exercise argument pass-through, not payload shape, so they hand + * the facade only the fields their assertions read. The registry contract asks + * for more (`vacay:invite` for a `from`, `notification:new` for a + * `notification`); filling that in would put data into the recorded call that + * nothing here checks, so the shortcut is named instead of padded out. + */ +function partialUserPayload( + payload: { type: E } & Partial>, +): { type: E } & TrekWsPayload { + return payload as { type: E } & TrekWsPayload; +} + beforeEach(() => vi.clearAllMocks()); describe('RealtimeService', () => { @@ -31,7 +45,7 @@ describe('RealtimeService', () => { }); it('broadcastToUser delegates every argument untouched', () => { - const payload = { type: 'vacay:invite', planId: 3 }; + const payload = partialUserPayload({ type: 'vacay:invite', planId: 3 }); svc.broadcastToUser(5, payload, '11'); expect(broadcastToUser).toHaveBeenCalledTimes(1); expect(broadcastToUser).toHaveBeenCalledWith(5, payload, '11'); @@ -45,7 +59,7 @@ describe('RealtimeService', () => { // routes through the mocked module functions rather than captured bindings. const late = new RealtimeService(); late.broadcast('1', 'todo:created', { item: {} }, undefined); - late.broadcastToUser(2, { type: 'notification:new' }); + late.broadcastToUser(2, partialUserPayload({ type: 'notification:new' })); expect(broadcast).toHaveBeenCalledWith('1', 'todo:created', { item: {} }, undefined); expect(broadcastToUser).toHaveBeenCalledWith(2, { type: 'notification:new' }); }); diff --git a/server/tests/unit/nest/trip-invite.service.test.ts b/server/tests/unit/nest/trip-invite.service.test.ts index 9bdbe2a69..06d62d25a 100644 --- a/server/tests/unit/nest/trip-invite.service.test.ts +++ b/server/tests/unit/nest/trip-invite.service.test.ts @@ -38,9 +38,13 @@ import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; import { TripInviteService } from '../../../src/nest/trip-invite/trip-invite.service'; -const svc = new TripInviteService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb))); +// One DatabaseService over the shared in-memory handle, so every collaborator +// reads and writes the same rows. +const dbs = new DatabaseService(testDb); +const svc = new TripInviteService(dbs, new PermissionsService(dbs), new TripMembershipService(dbs)); beforeAll(() => { createTables(testDb); runMigrations(testDb); }); beforeEach(() => resetTestDb(testDb)); diff --git a/server/tests/unit/plugins/host-rpc.test.ts b/server/tests/unit/plugins/host-rpc.test.ts index 7c540787f..1b2ae8d02 100644 --- a/server/tests/unit/plugins/host-rpc.test.ts +++ b/server/tests/unit/plugins/host-rpc.test.ts @@ -72,7 +72,9 @@ import type { LlmConfigResolver } from '../../../src/nest/llm-parse/llm-config.r import type { PluginOAuthService } from '../../../src/nest/plugins/oauth/plugin-oauth.service'; import type { RpcError, RpcResponse } from '../../../src/nest/plugins/protocol/envelope'; -const checkPermission = vi.fn(() => true as boolean); +// Typed from the real method rather than from the always-true body below, so a case that +// swaps in an implementation reading the action key (HOSTRPC-015) still type-checks. +const checkPermission = vi.fn(() => true); const permissions = { checkPermission } as unknown as PermissionsService; const addons = { isAddonEnabled: vi.fn(() => true) } as unknown as AddonsService; const notifications = { send: notifySend } as unknown as NotificationsService; @@ -310,8 +312,8 @@ describe('MetaRpc — namespaced entity metadata', () => { it('HOSTRPC-015 every entity type resolves its own edit action, and a read-only member is refused a write', async () => { const host = makeHost('meta', 'db:meta'); const seen: string[] = []; - checkPermission.mockImplementation((action: unknown) => { - seen.push(String(action)); + checkPermission.mockImplementation((action) => { + seen.push(action); return true; }); for (const [entityType, entityId] of [['trip', 1], ['place', 7], ['day', 3], ['reservation', 40], ['accommodation', 11]] as const) { diff --git a/server/tests/unit/plugins/plugin-guards.test.ts b/server/tests/unit/plugins/plugin-guards.test.ts index 86082c45b..2cea6fc06 100644 --- a/server/tests/unit/plugins/plugin-guards.test.ts +++ b/server/tests/unit/plugins/plugin-guards.test.ts @@ -12,10 +12,19 @@ import type { PermissionsService } from '../../../src/nest/permissions/permissio import type { AddonsService } from '../../../src/nest/addons/addons.service'; import type { PluginRpcContext } from '../../../src/nest/plugins/host/rpc-kit/types'; +/** + * The guards read `actingUserId` and nothing else off the context, so the two per-host + * collaborators are inert here. `plugins` gets real no-op functions rather than a cast, + * so a guard that ever reached for a peer would hit a callable, not undefined. + */ const ctx = (actingUserId: number | undefined): PluginRpcContext => ({ pluginId: 'p', actingUserId, data: {} as PluginRpcContext['data'], + plugins: { + call: vi.fn(async () => undefined), + emit: vi.fn(), + }, }); /** Trip 1 belongs to user 42 and only user 42 may reach it, mirroring rpc-host.test.ts. */ diff --git a/server/tests/unit/plugins/plugin-hooks.test.ts b/server/tests/unit/plugins/plugin-hooks.test.ts index af034129d..6255c0f6c 100644 --- a/server/tests/unit/plugins/plugin-hooks.test.ts +++ b/server/tests/unit/plugins/plugin-hooks.test.ts @@ -51,7 +51,10 @@ describe('PluginHooks contracts', () => { const registry = createTestPluginRegistry([new PluginHooks({} as never)]); it('PLUGHOOK-001 every hook the consent screen offers has a host-side consumer', () => { - const covered = new Set([...registry.hookContracts().values()].map((c) => c.hook)); + // A plain string set on purpose: the keys probed against it come from + // HOOK_PERMISSION, and whether each one is a hook the host consumes is the + // very thing under test, not something to assert into the element type. + const covered = new Set([...registry.hookContracts().values()].map((c) => c.hook)); expect([...Object.keys(HOOK_PERMISSION)].filter((h) => !covered.has(h))).toEqual([]); }); diff --git a/server/tests/unit/plugins/plugin-user-settings.test.ts b/server/tests/unit/plugins/plugin-user-settings.test.ts index 797b50b69..084a73a86 100644 --- a/server/tests/unit/plugins/plugin-user-settings.test.ts +++ b/server/tests/unit/plugins/plugin-user-settings.test.ts @@ -17,6 +17,7 @@ const { getDb } = vi.hoisted(() => ({ getDb: { current: null as unknown } })); vi.mock('../../../src/db/database', () => ({ get db() { return getDb.current; } })); import { db as dbConn } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import { AddonsService } from '../../../src/nest/addons/addons.service'; import Database from 'better-sqlite3'; import { PluginsService } from '../../../src/nest/plugins/plugins.service'; @@ -40,7 +41,13 @@ function freshDb() { describe('per-user plugin settings', () => { let svc: PluginsService; - beforeEach(() => { getDb.current = freshDb(); svc = new PluginsService(new DatabaseService(dbConn)); }); + // AddonsService only feeds PluginsService.list(), which no case here calls, but it is a + // real collaborator on the same connection rather than a stand-in. + beforeEach(() => { + getDb.current = freshDb(); + const dbs = new DatabaseService(dbConn); + svc = new PluginsService(dbs, new AddonsService(dbs)); + }); it('lists only the user-scope fields, in order', () => { const fields = svc.userSettingsFields('p'); diff --git a/server/tests/unit/plugins/plugins-service.test.ts b/server/tests/unit/plugins/plugins-service.test.ts index d214c76b5..3f1bc1ca4 100644 --- a/server/tests/unit/plugins/plugins-service.test.ts +++ b/server/tests/unit/plugins/plugins-service.test.ts @@ -118,7 +118,11 @@ describe('PluginsService.list', () => { it('never leaks the raw pinned key into the list response (only the fingerprint)', () => { insert('signed-one', 'acme/signed-one', 'RWTvBn0aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789abcd'); - const p = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins[0] as Record; + // PluginListItem is an interface, so it has no implicit index signature and cannot + // be narrowed to a record directly. Widening through unknown is what lets this case + // probe for a key the contract deliberately does not declare. + const p = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list() + .plugins[0] as unknown as Record; expect(p.author_pubkey).toBeUndefined(); }); }); diff --git a/server/tests/unit/plugins/rpc-host-inv-strip.test.ts b/server/tests/unit/plugins/rpc-host-inv-strip.test.ts index 824e4100d..9739dd507 100644 --- a/server/tests/unit/plugins/rpc-host-inv-strip.test.ts +++ b/server/tests/unit/plugins/rpc-host-inv-strip.test.ts @@ -18,11 +18,19 @@ import { TripsRpc } from '../../../src/nest/trips/trips.rpc'; import { DbRpc } from '../../../src/nest/plugins/host/rpc/db.rpc'; import { PluginGuards } from '../../../src/nest/plugins/host/plugin-guards.service'; import type { DatabaseService } from '../../../src/nest/database/database.service'; +import type { PluginUserSettingsService } from '../../../src/nest/plugins/plugin-user-settings.service'; const req = (method: string, params: Record): RpcRequest => ({ k: 'req', id: 'x', method, params }); -/** db.* is the cheapest handler to watch, and it needs no services of its own. */ -const dbRegistry = () => createTestPluginRegistry([new DbRpc()]); +/** + * db.* is the cheapest handler to watch. The only service it needs is the per-user + * settings store behind `settings.get`, which none of these cases dispatch, so the stub + * carries just that read, typed off the real signature. + */ +const dbRegistry = () => { + const settings: Pick = { readOne: () => undefined }; + return createTestPluginRegistry([new DbRpc(settings as PluginUserSettingsService)]); +}; /** trips.getById lives on the decorators now, so the audit cases bind it through them. */ const tripsRegistry = () => { diff --git a/server/tests/unit/plugins/rpc-host.test.ts b/server/tests/unit/plugins/rpc-host.test.ts index 0f320454d..ce30579fe 100644 --- a/server/tests/unit/plugins/rpc-host.test.ts +++ b/server/tests/unit/plugins/rpc-host.test.ts @@ -16,6 +16,14 @@ import type { RpcRequest, RpcResponse, RpcError } from '../../../src/nest/plugin import { makeDeps } from '../../helpers/rpc-host-deps'; const req = (method: string, params: Record = {}): RpcRequest => ({ k: 'req', id: 'x', method, params }); +/** + * A request as it arrives when the child never set `params`: the IPC hop is JSON, which + * drops an undefined field, so the key is genuinely absent by the time the router sees + * it. `RpcRequest` declares `params` as required, so leaving it out only type-checks + * through this cast — writing `params: undefined` instead would put the key back and + * stop covering the case the host's `req.params ?? {}` exists for. + */ +const reqWithoutParams = (method: string): RpcRequest => ({ k: 'req', id: 'x', method }) as RpcRequest; const ok = (r: RpcResponse | RpcError): r is RpcResponse => r.ok === true; const code = (r: RpcResponse | RpcError): string => (r as RpcError).error.code; @@ -158,7 +166,7 @@ describe('PluginRpcHost — capability enforcement', () => { seen = params; return null; }); - expect(ok(await host.dispatch({ k: 'req', id: 'x', method: 'tags.list' }, 42))).toBe(true); + expect(ok(await host.dispatch(reqWithoutParams('tags.list'), 42))).toBe(true); expect(seen).toEqual({}); }); diff --git a/server/tests/unit/plugins/rpc-kit/registry.test.ts b/server/tests/unit/plugins/rpc-kit/registry.test.ts index 06fd253a3..37872f4ec 100644 --- a/server/tests/unit/plugins/rpc-kit/registry.test.ts +++ b/server/tests/unit/plugins/rpc-kit/registry.test.ts @@ -20,6 +20,15 @@ const ctxFor = (actingUserId: number | undefined): PluginRpcContext => ({ pluginId: 'p', actingUserId, data: {} as PluginRpcContext['data'], + // No case below dispatches to a peer plugin, but the peers are a real collaborator + // on the context, so they get a stub that fails loudly rather than an empty object + // whose methods would be undefined the moment a handler reached for one. + plugins: { + call: () => Promise.reject(new Error('ctxFor: no peer plugin router in these cases')), + emit: () => { + throw new Error('ctxFor: no peer plugin router in these cases'); + }, + }, }); /** A well-formed controller: two tags methods with the permissions the table assigns. */