diff --git a/.github/workflows/native.yml b/.github/workflows/native.yml index f91c5d338..2bccc3c5a 100644 --- a/.github/workflows/native.yml +++ b/.github/workflows/native.yml @@ -121,18 +121,18 @@ jobs: working-directory: client run: npm run cap:sync -- ios - # SwiftPM leaves a half-downloaded binary target (Capacitor's xcframeworks) - # behind when a download from GitHub fails, and every later attempt then - # stops on "already exists in file system". Resolve on a clean artifact - # cache, up to three times, before the build needs the packages. + # Capacitor's xcframeworks come from GitHub release downloads, which answer + # 504 now and then; SwiftPM then reports the half-written file as "already + # exists in file system". Resolve on a clean artifact cache, a few times + # with growing pauses, before the build needs the packages. - name: Resolve the Swift packages working-directory: client/ios/App run: | - for attempt in 1 2 3; do + for attempt in 1 2 3 4 5; do rm -rf ~/Library/Caches/org.swift.swiftpm/artifacts if xcodebuild -resolvePackageDependencies -project App.xcodeproj -scheme App; then exit 0; fi - echo "Resolving the Swift packages failed (attempt $attempt of 3)" - sleep 20 + echo "Resolving the Swift packages failed (attempt $attempt of 5)" + sleep $((attempt * 30)) done exit 1 @@ -235,18 +235,18 @@ jobs: xcrun simctl list runtimes | grep -q watchOS || xcodebuild -downloadPlatform watchOS xcrun simctl list runtimes - # SwiftPM leaves a half-downloaded binary target (Capacitor's xcframeworks) - # behind when a download from GitHub fails, and every later attempt then - # stops on "already exists in file system". Resolve on a clean artifact - # cache, up to three times, before the build needs the packages. + # Capacitor's xcframeworks come from GitHub release downloads, which answer + # 504 now and then; SwiftPM then reports the half-written file as "already + # exists in file system". Resolve on a clean artifact cache, a few times + # with growing pauses, before the build needs the packages. - name: Resolve the Swift packages working-directory: client/ios/App run: | - for attempt in 1 2 3; do + for attempt in 1 2 3 4 5; do rm -rf ~/Library/Caches/org.swift.swiftpm/artifacts if xcodebuild -resolvePackageDependencies -project App.xcodeproj -scheme App; then exit 0; fi - echo "Resolving the Swift packages failed (attempt $attempt of 3)" - sleep 20 + echo "Resolving the Swift packages failed (attempt $attempt of 5)" + sleep $((attempt * 30)) done exit 1 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ed7e4a8c1..7ced5cb8c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1236,10 +1236,12 @@ jobs: bin="$RUNNER_TEMP/bin" mkdir -p "$bin" cd "$RUNNER_TEMP" - curl -sSfLo helm.tgz "https://get.helm.sh/helm-$HELM_VERSION-linux-amd64.tar.gz" + # GitHub's release downloads answer 504 now and then: retry, the checksum still decides. + fetch() { curl -sSfL --retry 5 --retry-delay 15 --retry-all-errors -o "$1" "$2"; } + fetch helm.tgz "https://get.helm.sh/helm-$HELM_VERSION-linux-amd64.tar.gz" echo "$HELM_SHA256 helm.tgz" | sha256sum -c - tar -xzf helm.tgz --strip-components=1 -C "$bin" linux-amd64/helm - curl -sSfLo kubeconform.tgz "https://github.com/yannh/kubeconform/releases/download/$KUBECONFORM_VERSION/kubeconform-linux-amd64.tar.gz" + fetch kubeconform.tgz "https://github.com/yannh/kubeconform/releases/download/$KUBECONFORM_VERSION/kubeconform-linux-amd64.tar.gz" echo "$KUBECONFORM_SHA256 kubeconform.tgz" | sha256sum -c - tar -xzf kubeconform.tgz -C "$bin" kubeconform echo "$bin" >> "$GITHUB_PATH" diff --git a/server/src/nest/notifications/notifications.module.ts b/server/src/nest/notifications/notifications.module.ts index fbe5471cb..30d49d879 100644 --- a/server/src/nest/notifications/notifications.module.ts +++ b/server/src/nest/notifications/notifications.module.ts @@ -4,11 +4,11 @@ import { Notifications } from '../../db/entities/Notifications.entity'; import { PushSubscriptions } from '../../db/entities/PushSubscriptions.entity'; import { Settings } from '../../db/entities/Settings.entity'; import { TodoItems } from '../../db/entities/TodoItems.entity'; -import { TripMembers } from '../../db/entities/TripMembers.entity'; import { Trips } from '../../db/entities/Trips.entity'; import { AddonsModule } from '../addons/addons.module'; import { SchedulingModule } from '../scheduling/scheduling.module'; import { StorageModule } from '../storage/storage.module'; +import { TripMembershipModule } from '../trip-membership/trip-membership.module'; import { MailerModule } from './mailer/mailer.module'; import { NotificationPreferencesService } from './notification-preferences.service'; import { AdminNotificationPreferencesController, NotificationsController } from './notifications.controller'; @@ -64,13 +64,13 @@ import { Module } from '@nestjs/common'; Trips, TodoItems, PushSubscriptions, - TripMembers, ]), AddonsModule, MailerModule, SchedulingModule, StorageModule, + TripMembershipModule, ], controllers: [NotificationsController, AdminNotificationPreferencesController, PushController], providers: [ diff --git a/server/src/nest/notifications/push/watch-sync-push.service.ts b/server/src/nest/notifications/push/watch-sync-push.service.ts index 43ff02833..e49cc84eb 100644 --- a/server/src/nest/notifications/push/watch-sync-push.service.ts +++ b/server/src/nest/notifications/push/watch-sync-push.service.ts @@ -1,11 +1,9 @@ -import { TripMembers } from '../../../db/entities/TripMembers.entity'; -import type { TripMembersRepository } from '../../../db/repositories/TripMembers.repository'; import { logError } from '../../audit/audit-log.logger'; import { withRequestContext } from '../../database/request-context'; import { RealtimeService } from '../../realtime/realtime.service'; +import { TripMembershipService } from '../../trip-membership/trip-membership.service'; import { WebPushService } from '../transports/web-push.service'; import { MikroORM } from '@mikro-orm/core'; -import { InjectRepository } from '@mikro-orm/nestjs'; import { Injectable, type OnModuleDestroy, type OnModuleInit } from '@nestjs/common'; /** @@ -46,7 +44,7 @@ export class WatchSyncPushService implements OnModuleInit, OnModuleDestroy { constructor( private readonly realtime: RealtimeService, private readonly push: WebPushService, - @InjectRepository(TripMembers) private readonly members: TripMembersRepository, + private readonly membership: TripMembershipService, private readonly orm: MikroORM, ) {} @@ -79,6 +77,17 @@ export class WatchSyncPushService implements OnModuleInit, OnModuleDestroy { else entry.users.add(onlyUserId); } + /** Everyone on the trip: the owner and the members. */ + private async roster(tripId: string): Promise> { + const [owner, members] = await Promise.all([ + this.membership.getOwnerId(tripId), + this.membership.listMemberUserIds(tripId), + ]); + const users = new Set(members); + if (owner != null) users.add(owner); + return users; + } + /** Sends one trip's gathered sync. */ async flush(key: string): Promise { const entry = this.pending.get(key); @@ -86,9 +95,7 @@ export class WatchSyncPushService implements OnModuleInit, OnModuleDestroy { this.pending.delete(key); clearTimeout(entry.timer); try { - const users = entry.everyone - ? await withRequestContext(this.orm, () => this.members.rosterUserIds(key)) - : entry.users; + const users = entry.everyone ? await withRequestContext(this.orm, () => this.roster(key)) : entry.users; await Promise.all([...users].map((userId) => this.push.sendWatchSync(userId))); } catch (err) { logError(`Watch sync failed trip=${key}: ${err instanceof Error ? err.message : err}`); diff --git a/server/tests/unit/nest/notifications/watch-sync-push.test.ts b/server/tests/unit/nest/notifications/watch-sync-push.test.ts index 6c8f63368..b78c7b735 100644 --- a/server/tests/unit/nest/notifications/watch-sync-push.test.ts +++ b/server/tests/unit/nest/notifications/watch-sync-push.test.ts @@ -1,91 +1,67 @@ /** * The watch sync (WSYNC-*): list changes on a trip become one silent push per - * member a moment later (WatchSyncPushService), sent to the iPhone app only and - * marked for the relay as a background push (WebPushService.sendWatchSync), and - * every trip broadcast reaches its listeners (RealtimeService.onTripBroadcast). + * member a moment later (WatchSyncPushService), and every trip broadcast + * reaches its listeners (RealtimeService.onTripBroadcast). The sending itself + * (WebPushService.sendWatchSync) is tested with the rest of the transport in + * web-push.service.test.ts. */ -import type { PushSubscriptionRow } from '../../../../src/db/repositories/PushSubscriptions.repository'; -import type { TripMembersRepository } from '../../../../src/db/repositories/TripMembers.repository'; -import type { PushSubscriptionsService } from '../../../../src/nest/notifications/push/push-subscriptions.service'; -import type { VapidKeysService } from '../../../../src/nest/notifications/push/vapid-keys.service'; import { WATCH_SYNC_SETTLE_MS, WatchSyncPushService, } from '../../../../src/nest/notifications/push/watch-sync-push.service'; -import { generateVapidKeyPair } from '../../../../src/nest/notifications/push/web-push-crypto'; -import { - BACKGROUND_PUSH_HEADER, - WebPushService, - isAppleRelayEndpoint, -} from '../../../../src/nest/notifications/transports/web-push.service'; +import { WebPushService } from '../../../../src/nest/notifications/transports/web-push.service'; import { RealtimeService } from '../../../../src/nest/realtime/realtime.service'; -import type { MikroORM } from '@mikro-orm/core'; +import { TripMembershipService } from '../../../../src/nest/trip-membership/trip-membership.service'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { createTestModule, type TestModule } from '../../../helpers/test-module'; -import { createECDH, randomBytes } from 'node:crypto'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const { logError, safeFetchFollow } = vi.hoisted(() => ({ logError: vi.fn(), safeFetchFollow: vi.fn() })); -vi.mock('../../../../src/nest/audit/audit-log.logger', () => ({ - LOG_LEVEL: 'error', - logInfo: vi.fn(), - logDebug: vi.fn(), - logError, - logWarn: vi.fn(), -})); -vi.mock('../../../../src/utils/ssrfGuard', () => ({ - SsrfBlockedError: class SsrfBlockedError extends Error {}, - safeFetchFollow, -})); -// The repository's read runs inside a request context; the fake repository needs none. -vi.mock('../../../../src/nest/database/request-context', () => ({ - withRequestContext: (_orm: unknown, fn: () => unknown) => fn(), -})); -// The socket registry has no rooms here; only the listeners matter. -vi.mock('../../../../src/nest/realtime/ws-state', () => ({ - broadcast: vi.fn(), - broadcastToUser: vi.fn(), - getOnlineUserIds: () => new Set(), -})); - -const APNS = 'https://push.liketrek.com/v1/apns/production/' + 'ab'.repeat(32); -const FCM = 'https://push.liketrek.com/v1/fcm/' + 'x'.repeat(140); -const BROWSER = 'https://fcm.googleapis.com/fcm/send/abc'; +const db = createSnapshotTestDb(); describe('WatchSyncPushService', () => { + let t: TestModule; let realtime: RealtimeService; - let sendWatchSync: ReturnType; - let rosterUserIds: ReturnType; let service: WatchSyncPushService; + const sendWatchSync = vi.fn(); + const getOwnerId = vi.fn(); + const listMemberUserIds = vi.fn(); - beforeEach(() => { - vi.useFakeTimers(); - logError.mockClear(); + beforeEach(async () => { + sendWatchSync.mockReset().mockResolvedValue(true); + getOwnerId.mockReset().mockResolvedValue(1); + listMemberUserIds.mockReset().mockResolvedValue([2, 3]); realtime = new RealtimeService(); - sendWatchSync = vi.fn().mockResolvedValue(true); - rosterUserIds = vi.fn().mockResolvedValue(new Set([1, 2, 3])); - service = new WatchSyncPushService( + t = await createTestModule({ + db, + imports: [], realtime, - { sendWatchSync } as unknown as WebPushService, - { rosterUserIds } as unknown as TripMembersRepository, - {} as MikroORM, - ); - service.onModuleInit(); + providers: [ + WatchSyncPushService, + { provide: WebPushService, useValue: { sendWatchSync } }, + { provide: TripMembershipService, useValue: { getOwnerId, listMemberUserIds } }, + ], + init: true, + }); + service = t.get(WatchSyncPushService); + // Only now: the module compiles on real timers. + vi.useFakeTimers(); }); - afterEach(() => { - service.onModuleDestroy(); + afterEach(async () => { vi.useRealTimers(); + await t.close(); }); - it('WSYNC-001: a run of list changes on a trip becomes one sync to every member, once it settled', async () => { + it('WSYNC-001: a run of list changes on a trip becomes one sync to its owner and members, once it settled', async () => { realtime.broadcast(12, 'packing:updated', { item: {} } as never); realtime.broadcast(12, 'todo:updated', { item: {} } as never); realtime.broadcast(12, 'packing:deleted', { itemId: 4 } as never); expect(sendWatchSync).not.toHaveBeenCalled(); await vi.advanceTimersByTimeAsync(WATCH_SYNC_SETTLE_MS); - expect(rosterUserIds).toHaveBeenCalledTimes(1); - expect(rosterUserIds).toHaveBeenCalledWith('12'); + expect(listMemberUserIds).toHaveBeenCalledTimes(1); + expect(listMemberUserIds).toHaveBeenCalledWith('12'); expect(sendWatchSync.mock.calls.map(([id]) => id).sort()).toEqual([1, 2, 3]); }); @@ -99,7 +75,7 @@ describe('WatchSyncPushService', () => { it('WSYNC-003: a private packing item syncs its owner alone, unless the trip changed for everyone too', async () => { realtime.broadcast(12, 'packing:updated', { item: {} } as never, undefined, 2); await vi.advanceTimersByTimeAsync(WATCH_SYNC_SETTLE_MS); - expect(rosterUserIds).not.toHaveBeenCalled(); + expect(listMemberUserIds).not.toHaveBeenCalled(); expect(sendWatchSync.mock.calls).toEqual([[2]]); sendWatchSync.mockClear(); @@ -109,18 +85,19 @@ describe('WatchSyncPushService', () => { expect(sendWatchSync.mock.calls.map(([id]) => id).sort()).toEqual([1, 2, 3]); }); - it('WSYNC-004: each trip gathers on its own', async () => { + it('WSYNC-004: each trip gathers on its own, and a trip without an owner row still reaches its members', async () => { + getOwnerId.mockResolvedValueOnce(null); realtime.broadcast(12, 'todo:updated', { item: {} } as never); realtime.broadcast(13, 'todo:updated', { item: {} } as never); await vi.advanceTimersByTimeAsync(WATCH_SYNC_SETTLE_MS); - expect(rosterUserIds.mock.calls).toEqual([['12'], ['13']]); + expect(listMemberUserIds.mock.calls).toEqual([['12'], ['13']]); + expect(sendWatchSync).toHaveBeenCalledTimes(5); }); - it('WSYNC-005: a failed read is logged and never thrown into the broadcast', async () => { - rosterUserIds.mockRejectedValue(new Error('db gone')); + it('WSYNC-005: a failed read is swallowed with a log line and never thrown into the broadcast', async () => { + listMemberUserIds.mockRejectedValue(new Error('db gone')); realtime.broadcast(12, 'todo:updated', { item: {} } as never); await vi.advanceTimersByTimeAsync(WATCH_SYNC_SETTLE_MS); - expect(logError).toHaveBeenCalledWith(expect.stringContaining('Watch sync failed trip=12: db gone')); expect(sendWatchSync).not.toHaveBeenCalled(); }); @@ -150,85 +127,3 @@ describe('RealtimeService.onTripBroadcast', () => { expect(heard).toHaveBeenCalledTimes(1); }); }); - -describe('WebPushService.sendWatchSync', () => { - const keys = generateVapidKeyPair(); - - function row(id: number, endpoint: string): PushSubscriptionRow { - const device = createECDH('prime256v1'); - device.generateKeys(); - return { - id, - user_id: 5, - endpoint, - p256dh: device.getPublicKey().toString('base64url'), - auth: randomBytes(16).toString('base64url'), - vapid_public_key: keys.publicKey, - user_agent: null, - created_at: '2026-11-08T00:00:00Z', - last_success_at: null, - failure_count: 0, - }; - } - - function service(rows: PushSubscriptionRow[], available = true) { - const vapid = { - isAvailable: vi.fn().mockResolvedValue(available), - getKeys: vi.fn().mockResolvedValue({ ...keys, source: 'database' }), - getSubject: () => 'mailto:admin@trek.example.test', - } as unknown as VapidKeysService; - const subscriptions = { - listForUser: vi.fn().mockResolvedValue(rows), - recordSuccess: vi.fn(), - recordFailure: vi.fn().mockResolvedValue(1), - deleteById: vi.fn(), - } as unknown as PushSubscriptionsService; - return new WebPushService(vapid, subscriptions); - } - - beforeEach(() => { - safeFetchFollow.mockReset().mockResolvedValue(new Response(null, { status: 201 })); - }); - - it('WSYNC-008: only the iPhone app gets it, as a short-lived background push the relay recognises', async () => { - const sent = await service([row(1, APNS), row(2, FCM), row(3, BROWSER)]).sendWatchSync(5); - expect(sent).toBe(true); - expect(safeFetchFollow).toHaveBeenCalledTimes(1); - const [endpoint, init] = safeFetchFollow.mock.calls[0]; - expect(endpoint).toBe(APNS); - expect(init.headers).toMatchObject({ - [BACKGROUND_PUSH_HEADER]: 'background', - Topic: 'trek-watch-sync', - Urgency: 'low', - TTL: '600', - 'Content-Encoding': 'aes128gcm', - }); - }); - - it('WSYNC-009: a user without the iPhone app, or a server without keys, sends nothing', async () => { - expect(await service([row(2, FCM), row(3, BROWSER)]).sendWatchSync(5)).toBe(false); - expect(await service([row(1, APNS)], false).sendWatchSync(5)).toBe(false); - expect(safeFetchFollow).not.toHaveBeenCalled(); - }); - - it('WSYNC-010: an ordinary notification still goes out without the background mark', async () => { - await service([row(1, APNS)]).sendToUser(5, { - event: 'collab_message', - title: 'Hi', - body: 'there', - navigateTarget: '/trips/1', - url: 'https://trek.example.test/trips/1', - }); - const [, init] = safeFetchFollow.mock.calls[0]; - expect(init.headers[BACKGROUND_PUSH_HEADER]).toBeUndefined(); - expect(init.headers.TTL).toBe(String(24 * 60 * 60)); - }); - - it("WSYNC-011: only the relay's APNs endpoints count as an iPhone", () => { - expect(isAppleRelayEndpoint(APNS)).toBe(true); - expect(isAppleRelayEndpoint(FCM)).toBe(false); - expect(isAppleRelayEndpoint(BROWSER)).toBe(false); - expect(isAppleRelayEndpoint('http://push.liketrek.com/v1/apns/production/ab')).toBe(false); - expect(isAppleRelayEndpoint('not a url')).toBe(false); - }); -}); diff --git a/server/tests/unit/nest/notifications/web-push.service.test.ts b/server/tests/unit/nest/notifications/web-push.service.test.ts index 3547c751a..d5aa22530 100644 --- a/server/tests/unit/nest/notifications/web-push.service.test.ts +++ b/server/tests/unit/nest/notifications/web-push.service.test.ts @@ -18,10 +18,12 @@ import { } from '../../../../src/nest/notifications/push/vapid-keys.service'; import { generateVapidKeyPair } from '../../../../src/nest/notifications/push/web-push-crypto'; import { + BACKGROUND_PUSH_HEADER, FORBIDDEN_FAILURES_BEFORE_REMOVAL, MAX_PUSH_PAYLOAD_BYTES, WebPushService, buildPushPayload, + isAppleRelayEndpoint, samePagePath, } from '../../../../src/nest/notifications/transports/web-push.service'; import { SsrfBlockedError } from '../../../../src/utils/ssrfGuard'; @@ -469,6 +471,57 @@ describe('WebPushService delivery', () => { }); }); +describe('WebPushService.sendWatchSync', () => { + const APNS = `https://push.liketrek.com/v1/apns/production/${'ab'.repeat(32)}`; + const FCM = `https://push.liketrek.com/v1/fcm/${'x'.repeat(140)}`; + const BROWSER = 'https://fcm.googleapis.com/fcm/send/watch-sync-browser'; + + it('WSYNC-008: only the iPhone app gets it, as a short-lived background push the relay recognises', async () => { + const { user } = createUser(testDb); + await subscribe(user.id, APNS); + await subscribe(user.id, FCM); + await subscribe(user.id, BROWSER); + safeFetchFollow.mockResolvedValue(reply(201)); + + expect(await push.sendWatchSync(user.id)).toBe(true); + expect(safeFetchFollow).toHaveBeenCalledTimes(1); + const [url, init] = safeFetchFollow.mock.calls[0]; + expect(url).toBe(APNS); + expect(init.headers).toMatchObject({ + [BACKGROUND_PUSH_HEADER]: 'background', + Topic: 'trek-watch-sync', + Urgency: 'low', + TTL: '600', + 'Content-Encoding': 'aes128gcm', + }); + }); + + it('WSYNC-009: a user without the iPhone app sends nothing', async () => { + const { user } = createUser(testDb); + await subscribe(user.id, BROWSER); + expect(await push.sendWatchSync(user.id)).toBe(false); + expect(safeFetchFollow).not.toHaveBeenCalled(); + }); + + it('WSYNC-010: an ordinary notification to the same app goes out without the background mark', async () => { + const { user } = createUser(testDb); + await subscribe(user.id, APNS); + safeFetchFollow.mockResolvedValue(reply(201)); + await push.sendToUser(user.id, MSG); + const [, init] = safeFetchFollow.mock.calls[0]; + expect(init.headers[BACKGROUND_PUSH_HEADER]).toBeUndefined(); + expect(init.headers.TTL).toBe(String(24 * 60 * 60)); + }); + + it("WSYNC-011: only the relay's APNs endpoints count as an iPhone", () => { + expect(isAppleRelayEndpoint(APNS)).toBe(true); + expect(isAppleRelayEndpoint(FCM)).toBe(false); + expect(isAppleRelayEndpoint(BROWSER)).toBe(false); + expect(isAppleRelayEndpoint(`http://push.liketrek.com/v1/apns/production/${'ab'.repeat(32)}`)).toBe(false); + expect(isAppleRelayEndpoint('not a url')).toBe(false); + }); +}); + describe('buildPushPayload', () => { const size = (p: unknown) => Buffer.byteLength(JSON.stringify(p), 'utf8');