fix(security): scope connect modify() guest write (review follow-up to #126)

apps/api/src/modules/connect/connect-booking.service.spec.ts

@@ -419,6 +419,71 @@ describe('ConnectBookingService', () => {
       expect(mockAvailabilityService.searchAvailability).toHaveBeenCalled();
     });
 
+    it('forks a property-local guest on name change when the guest is shared with another property', async () => {
+      let selectCallCount = 0;
+      mockDb.select.mockImplementation(() => ({
+        from: vi.fn().mockReturnValue({
+          where: vi.fn().mockImplementation(() => {
+            selectCallCount++;
+            if (selectCallCount === 1) return Promise.resolve([{ id: 'booking-1', propertyId: 'prop-1' }]);
+            if (selectCallCount === 2) return Promise.resolve([{
+              id: 'res-1', bookingId: 'booking-1', guestId: 'guest-shared',
+              status: 'confirmed', totalAmount: '399.98',
+              arrivalDate: '2024-06-01', departureDate: '2024-06-03',
+            }]);
+            if (selectCallCount === 3) return Promise.resolve([{ id: 'res-other' }]); // guest linked at ANOTHER property
+            if (selectCallCount === 4) return Promise.resolve([{ id: 'guest-shared', firstName: 'John', lastName: 'Smith', email: 'j@x.com' }]);
+            return Promise.resolve([]);
+          }),
+        }),
+      }));
+
+      let insertCount = 0;
+      mockDb.insert.mockImplementation(() => ({
+        values: vi.fn().mockReturnValue({
+          returning: vi.fn().mockImplementation(() => {
+            insertCount++;
+            return Promise.resolve([{ id: 'guest-forked', firstName: 'Johnny', lastName: 'Smith' }]);
+          }),
+        }),
+      }));
+
+      const result = await service.modify('HAIP-123', { guestFirstName: 'Johnny' });
+
+      expect(result.success).toBe(true);
+      // A shared guest must NOT be overwritten in place — a property-local copy is forked.
+      expect(insertCount).toBe(1);
+    });
+
+    it('updates the guest in place on name change when the guest is NOT shared', async () => {
+      let selectCallCount = 0;
+      mockDb.select.mockImplementation(() => ({
+        from: vi.fn().mockReturnValue({
+          where: vi.fn().mockImplementation(() => {
+            selectCallCount++;
+            if (selectCallCount === 1) return Promise.resolve([{ id: 'booking-1', propertyId: 'prop-1' }]);
+            if (selectCallCount === 2) return Promise.resolve([{
+              id: 'res-1', bookingId: 'booking-1', guestId: 'guest-1',
+              status: 'confirmed', totalAmount: '399.98',
+              arrivalDate: '2024-06-01', departureDate: '2024-06-03',
+            }]);
+            if (selectCallCount === 3) return Promise.resolve([]); // no other-property links → not shared
+            return Promise.resolve([]);
+          }),
+        }),
+      }));
+
+      let insertCount = 0;
+      mockDb.insert.mockImplementation(() => ({
+        values: vi.fn().mockReturnValue({ returning: vi.fn().mockImplementation(() => { insertCount++; return Promise.resolve([{ id: 'x' }]); }) }),
+      }));
+
+      const result = await service.modify('HAIP-123', { guestFirstName: 'Johnny' });
+
+      expect(result.success).toBe(true);
+      expect(insertCount).toBe(0); // updated in place, no fork
+    });
+
     it('should reject modification of cancelled reservation', async () => {
       let selectCallCount = 0;
       mockDb.select.mockImplementation(() => ({

apps/api/src/modules/connect/connect-booking.service.ts

@@ -1,5 +1,5 @@
 import { Injectable, Inject, NotFoundException, BadRequestException } from '@nestjs/common';
-import { eq, and } from 'drizzle-orm';
+import { eq, and, ne } from 'drizzle-orm';
 import Decimal from 'decimal.js';
 import { bookings, reservations, guests, ratePlans, roomTypes, folios, rooms } from '@telivityhaip/database';
 import { DRIZZLE } from '../../database/database.module';
@@ -263,15 +263,52 @@ export class ConnectBookingService {
     const previousAmountDec = new Decimal(reservation.totalAmount);
     const previousAmount = previousAmountDec.toNumber();
 
-    // Handle guest detail updates
+    // Handle guest detail updates. The `guests` row is cross-property by design,
+    // so overwriting it in place would corrupt the profile as seen by OTHER
+    // properties that share this guest. Only mutate in place when the guest is
+    // NOT linked to any other property; otherwise fork a property-local copy and
+    // repoint this reservation, leaving the shared row untouched.
     if (dto.guestFirstName || dto.guestLastName) {
       const guestUpdate: Record<string, any> = {};
       if (dto.guestFirstName) guestUpdate['firstName'] = dto.guestFirstName;
       if (dto.guestLastName) guestUpdate['lastName'] = dto.guestLastName;
-      await this.db
-        .update(guests)
-        .set(guestUpdate)
-        .where(eq(guests.id, reservation.guestId));
+
+      const otherPropertyLinks = await this.db
+        .select({ id: reservations.id })
+        .from(reservations)
+        .where(
+          and(
+            eq(reservations.guestId, reservation.guestId),
+            ne(reservations.propertyId, booking.propertyId),
+          ),
+        );
+
+      if (otherPropertyLinks.length > 0) {
+        const [current] = await this.db
+          .select()
+          .from(guests)
+          .where(eq(guests.id, reservation.guestId));
+        const [forked] = await this.db
+          .insert(guests)
+          .values({
+            firstName: guestUpdate['firstName'] ?? current?.firstName,
+            lastName: guestUpdate['lastName'] ?? current?.lastName,
+            email: current?.email ?? null,
+            phone: current?.phone ?? null,
+            loyaltyNumber: current?.loyaltyNumber ?? null,
+          })
+          .returning();
+        await this.db
+          .update(reservations)
+          .set({ guestId: forked.id, updatedAt: new Date() })
+          .where(eq(reservations.id, reservation.id));
+        reservation.guestId = forked.id;
+      } else {
+        await this.db
+          .update(guests)
+          .set(guestUpdate)
+          .where(eq(guests.id, reservation.guestId));
+      }
     }
 
     // Handle simple field updates