From e5b021b478151d44a246a160b487abb97ec2a5e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Mili=C4=87evi=C4=87?= Date: Fri, 19 Jun 2026 13:13:32 -0500 Subject: [PATCH 1/5] fix(security): booking-engine price-pairing, redact webhook secret, scope guest reuse by property - booking-engine quote/book: reject rate plan whose roomTypeId != requested roomType (price manipulation via independently-sellable room/rate ids) - connect-events: never return the HMAC subscription secret in create/list responses - connect + OTA inbound findOrCreateGuest: only reuse a guest already linked to the requesting property; otherwise create a fresh row (cross-tenant PII leak/poison) Co-Authored-By: Claude Opus 4.8 --- .../booking-engine.service.spec.ts | 19 +++++++ .../booking-engine/booking-engine.service.ts | 10 ++++ .../inbound-reservation.service.spec.ts | 26 ++++++++++ .../channel/inbound-reservation.service.ts | 48 +++++++---------- .../connect/connect-booking.service.spec.ts | 52 ++++++++++++++++++- .../connect/connect-booking.service.ts | 21 ++++++-- .../connect/connect-events.service.spec.ts | 25 +++++++++ .../modules/connect/connect-events.service.ts | 16 +++++- 8 files changed, 181 insertions(+), 36 deletions(-) diff --git a/apps/api/src/modules/booking-engine/booking-engine.service.spec.ts b/apps/api/src/modules/booking-engine/booking-engine.service.spec.ts index 7ffa4db..4a38d2c 100644 --- a/apps/api/src/modules/booking-engine/booking-engine.service.spec.ts +++ b/apps/api/src/modules/booking-engine/booking-engine.service.spec.ts @@ -24,6 +24,7 @@ function makeService(overrides: Partial> = {}) { const ratePlan = { calculateDerivedRate: vi.fn().mockResolvedValue({ effectiveRate: 100, currency: 'USD' }), assertSellable: vi.fn().mockResolvedValue(undefined), + findById: vi.fn().mockResolvedValue({ id: RP, roomTypeId: RT, currencyCode: 'USD' }), }; const tax = { calculateTaxes: vi.fn().mockResolvedValue([{ amount: '10.00' }]) }; const guest = { create: vi.fn().mockResolvedValue({ id: 'guest-1' }) }; @@ -154,4 +155,22 @@ describe('BookingEngineService.book', () => { const { paymentToken, ...noToken } = bookDto as any; await expect(svc.book(PROP, noToken)).rejects.toBeInstanceOf(BadRequestException); }); + + it('rejects a rate plan that belongs to a different room type (price-tampering guard)', async () => { + // Attacker pairs a pricey room type with a cheap room's rate plan. Both are + // individually sellable, but the rate plan is bound to a DIFFERENT room type. + const { svc, ratePlan } = makeService(); + ratePlan.findById.mockResolvedValue({ id: RP, roomTypeId: 'rt000000-0000-4000-a000-0000000000ff', currencyCode: 'USD' }); + await expect(svc.book(PROP, bookDto as any)).rejects.toBeInstanceOf(BadRequestException); + }); +}); + +describe('BookingEngineService.quote — rate/room pairing', () => { + it('rejects a rate plan that does not belong to the requested room type', async () => { + const { svc, ratePlan } = makeService(); + ratePlan.findById.mockResolvedValue({ id: RP, roomTypeId: 'rt000000-0000-4000-a000-0000000000ff', currencyCode: 'USD' }); + await expect( + svc.quote(PROP, { roomTypeId: RT, ratePlanId: RP, checkIn: '2026-07-01', checkOut: '2026-07-03', adults: 2 } as any), + ).rejects.toBeInstanceOf(BadRequestException); + }); }); diff --git a/apps/api/src/modules/booking-engine/booking-engine.service.ts b/apps/api/src/modules/booking-engine/booking-engine.service.ts index d4da2b2..ea8fc50 100644 --- a/apps/api/src/modules/booking-engine/booking-engine.service.ts +++ b/apps/api/src/modules/booking-engine/booking-engine.service.ts @@ -98,6 +98,16 @@ export class BookingEngineService { const config = await this.configService.getPublicConfig(propertyId); this.assertSellable(config, dto.roomTypeId, dto.ratePlanId); + // Price-tampering guard: `roomTypeId` and `ratePlanId` arrive as two + // independent client-supplied ids. `assertSellable` only checks each id is + // individually sellable, so a caller could pair a pricey room type with a + // cheap room's rate plan and be charged the cheap rate. Each rate plan is + // bound to exactly one room type — enforce that they match. + const ratePlanRow = await this.ratePlanService.findById(dto.ratePlanId, propertyId); + if (ratePlanRow.roomTypeId !== dto.roomTypeId) { + throw new BadRequestException('Rate plan does not apply to the selected room type'); + } + const nights = this.nightsBetween(dto.checkIn, dto.checkOut); // Re-confirm availability for the requested room type. diff --git a/apps/api/src/modules/channel/inbound-reservation.service.spec.ts b/apps/api/src/modules/channel/inbound-reservation.service.spec.ts index 2ca1051..4de443f 100644 --- a/apps/api/src/modules/channel/inbound-reservation.service.spec.ts +++ b/apps/api/src/modules/channel/inbound-reservation.service.spec.ts @@ -326,6 +326,7 @@ describe('InboundReservationService', () => { if (callCount === 3) return Promise.resolve([{ id: 'rt-1' }]); // roomTypes FK OK if (callCount === 4) return Promise.resolve([{ id: 'rp-1' }]); // ratePlans FK OK if (callCount === 5) return Promise.resolve([existingGuest]); // existing guest by email + if (callCount === 6) return Promise.resolve([{ id: 'res-link' }]); // linked at THIS property → reuse return Promise.resolve([]); }), }), @@ -345,5 +346,30 @@ describe('InboundReservationService', () => { expect(result.guestId).toBe('guest-existing'); }); + + it('should NOT reuse a guest with no reservation link at this property', async () => { + let callCount = 0; + const foreignGuest = { id: 'guest-foreign', firstName: 'John', email: 'john@example.com' }; + mockDb.select.mockImplementation(() => ({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockImplementation(() => { + callCount++; + if (callCount === 1) return Promise.resolve([mockConnection]); + if (callCount === 2) return Promise.resolve([]); // no existing booking + if (callCount === 3) return Promise.resolve([{ id: 'rt-1' }]); // roomTypes FK OK + if (callCount === 4) return Promise.resolve([{ id: 'rp-1' }]); // ratePlans FK OK + if (callCount === 5) return Promise.resolve([foreignGuest]); // email matches a guest... + if (callCount === 6) return Promise.resolve([]); // ...but NO reservation link at this property + return Promise.resolve([]); + }), + }), + })); + + const reservation = makeReservation(); + const result = await service.processInboundReservation('conn-1', reservation); + + // A fresh guest row is created (beforeEach insert mock → 'guest-1'), not the foreign one. + expect(result.guestId).toBe('guest-1'); + }); }); }); diff --git a/apps/api/src/modules/channel/inbound-reservation.service.ts b/apps/api/src/modules/channel/inbound-reservation.service.ts index a3ae35d..08a1bf0 100644 --- a/apps/api/src/modules/channel/inbound-reservation.service.ts +++ b/apps/api/src/modules/channel/inbound-reservation.service.ts @@ -122,7 +122,7 @@ export class InboundReservationService { // Atomically create guest + booking + reservation so we never end up with a half-written record. const { guest, booking, pmsReservation } = await this.db.transaction(async (tx: any) => { - const guest = await this.findOrCreateGuestTx(tx, reservation); + const guest = await this.findOrCreateGuestTx(tx, reservation, propertyId); const [booking] = await tx .insert(bookings) @@ -398,13 +398,28 @@ export class InboundReservationService { return existing ?? null; } - private async findOrCreateGuestTx(tx: any, reservation: ChannelReservation) { + private async findOrCreateGuestTx(tx: any, reservation: ChannelReservation, propertyId: string) { + // Reuse a guest by email ONLY when that guest is already linked to THIS + // property via an existing reservation. A bare cross-property email match + // would let an inbound OTA push attach to (and corrupt) another tenant's + // guest profile. Otherwise create a fresh row (CLAUDE.md guest rule). if (reservation.guestEmail) { - const [existing] = await tx + const matches = await tx .select() .from(guests) .where(eq(guests.email, reservation.guestEmail)); - if (existing) return existing; + for (const candidate of matches) { + const links = await tx + .select({ id: reservations.id }) + .from(reservations) + .where( + and( + eq(reservations.guestId, candidate.id), + eq(reservations.propertyId, propertyId), + ), + ); + if (links.length > 0) return candidate; + } } const [guest] = await tx .insert(guests) @@ -418,31 +433,6 @@ export class InboundReservationService { return guest; } - private async findOrCreateGuest(reservation: ChannelReservation) { - // Try to find existing guest by email - if (reservation.guestEmail) { - const [existing] = await this.db - .select() - .from(guests) - .where(eq(guests.email, reservation.guestEmail)); - - if (existing) return existing; - } - - // Create new guest - const [guest] = await this.db - .insert(guests) - .values({ - firstName: reservation.guestFirstName, - lastName: reservation.guestLastName, - email: reservation.guestEmail ?? null, - phone: reservation.guestPhone ?? null, - }) - .returning(); - - return guest; - } - private resolveRoomTypeId(conn: any, channelRoomCode: string): string { const roomTypeMapping = (conn.roomTypeMapping ?? []) as Array<{ roomTypeId: string; diff --git a/apps/api/src/modules/connect/connect-booking.service.spec.ts b/apps/api/src/modules/connect/connect-booking.service.spec.ts index 775e886..63c017a 100644 --- a/apps/api/src/modules/connect/connect-booking.service.spec.ts +++ b/apps/api/src/modules/connect/connect-booking.service.spec.ts @@ -107,8 +107,9 @@ describe('ConnectBookingService', () => { where: vi.fn().mockImplementation(() => { selectCallCount++; if (selectCallCount === 1) return Promise.resolve([mockRatePlan]); - if (selectCallCount === 2) return Promise.resolve([existingGuest]); // guest found! - if (selectCallCount === 3) return Promise.resolve([{ settings: {} }]); + if (selectCallCount === 2) return Promise.resolve([existingGuest]); // guest found by email + if (selectCallCount === 3) return Promise.resolve([{ id: 'res-existing' }]); // linked at THIS property → reuse + if (selectCallCount === 4) return Promise.resolve([{ settings: {} }]); return Promise.resolve([]); }), }), @@ -144,6 +145,53 @@ describe('ConnectBookingService', () => { expect(insertCount).toBe(2); }); + it('should NOT reuse a guest from another property (cross-tenant PII guard)', async () => { + let selectCallCount = 0; + const foreignGuest = { id: 'guest-foreign', firstName: 'John', lastName: 'Smith', email: 'john@example.com' }; + mockDb.select.mockImplementation(() => ({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockImplementation(() => { + selectCallCount++; + if (selectCallCount === 1) return Promise.resolve([mockRatePlan]); // rate plan + if (selectCallCount === 2) return Promise.resolve([foreignGuest]); // email matches a guest... + if (selectCallCount === 3) return Promise.resolve([]); // ...but NO reservation link at this property + if (selectCallCount === 4) return Promise.resolve([{ settings: {} }]); // property settings + return Promise.resolve([]); + }), + }), + })); + + let insertCount = 0; + mockDb.insert.mockImplementation(() => ({ + values: vi.fn().mockReturnValue({ + returning: vi.fn().mockImplementation(() => { + insertCount++; + if (insertCount === 1) return Promise.resolve([{ id: 'guest-new', firstName: 'John', lastName: 'Smith' }]); + if (insertCount === 2) return Promise.resolve([{ id: 'booking-1', confirmationNumber: 'HAIP-X' }]); + if (insertCount === 3) return Promise.resolve([{ id: 'res-1', status: 'confirmed' }]); + return Promise.resolve([{}]); + }), + }), + })); + + const result = await service.book({ + propertyId: 'prop-1', + roomTypeId: 'rt-1', + ratePlanId: 'rp-1', + checkIn: '2024-06-01', + checkOut: '2024-06-03', + guestFirstName: 'John', + guestLastName: 'Smith', + guestEmail: 'john@example.com', + adults: 2, + }); + + expect(result.success).toBe(true); + // A fresh guest row is created (guest + booking + reservation = 3 inserts), + // NOT linked to the foreign-property guest. + expect(insertCount).toBe(3); + }); + it('should reject booking when no availability', async () => { mockAvailabilityService.searchAvailability.mockResolvedValue([ { roomTypeId: 'rt-1', date: '2024-06-01', totalRooms: 50, sold: 50, available: 0, overbookingBuffer: 0 }, diff --git a/apps/api/src/modules/connect/connect-booking.service.ts b/apps/api/src/modules/connect/connect-booking.service.ts index 8456731..e4c5622 100644 --- a/apps/api/src/modules/connect/connect-booking.service.ts +++ b/apps/api/src/modules/connect/connect-booking.service.ts @@ -439,13 +439,28 @@ export class ConnectBookingService { // --- Private Helpers --- private async findOrCreateGuest(dto: AgentBookDto) { - // Try to find by email + // Try to find by email — but ONLY reuse a guest that is already linked to + // THIS property via an existing reservation. The `guests` row is cross-property + // by design, yet a bare email match would let one tenant attach to (and later, + // via modify(), overwrite) another tenant's guest profile. Scope reuse to the + // requesting property; otherwise create a fresh row (CLAUDE.md guest rule). if (dto.guestEmail) { - const [existing] = await this.db + const matches = await this.db .select() .from(guests) .where(eq(guests.email, dto.guestEmail)); - if (existing) return existing; + for (const candidate of matches) { + const links = await this.db + .select({ id: reservations.id }) + .from(reservations) + .where( + and( + eq(reservations.guestId, candidate.id), + eq(reservations.propertyId, dto.propertyId), + ), + ); + if (links.length > 0) return candidate; + } } // Create new guest diff --git a/apps/api/src/modules/connect/connect-events.service.spec.ts b/apps/api/src/modules/connect/connect-events.service.spec.ts index 0dc1469..db70fc4 100644 --- a/apps/api/src/modules/connect/connect-events.service.spec.ts +++ b/apps/api/src/modules/connect/connect-events.service.spec.ts @@ -13,6 +13,7 @@ describe('ConnectEventsService', () => { subscriberName: 'OTAIP Booking Agent', callbackUrl: 'https://otaip.example.com/webhooks', events: ['reservation.*', 'folio.charge_posted'], + secret: 'whsec_supersecret', isActive: true, failureCount: 0, }; @@ -55,6 +56,18 @@ describe('ConnectEventsService', () => { expect(result.id).toBe('sub-1'); expect(result.subscriberId).toBe('otaip-agent-v1'); }); + + it('never returns the HMAC secret in the response', async () => { + const result = await service.createSubscription({ + propertyId: 'prop-1', + subscriberId: 'otaip-agent-v1', + callbackUrl: 'https://otaip.example.com/webhooks', + events: ['reservation.*'], + secret: 'whsec_supersecret', + } as any); + + expect((result as any).secret).toBeUndefined(); + }); }); describe('listSubscriptions', () => { @@ -70,6 +83,18 @@ describe('ConnectEventsService', () => { expect(result).toHaveLength(1); expect(result[0]!.subscriberId).toBe('otaip-agent-v1'); }); + + it('never returns the HMAC secret in listed subscriptions', async () => { + mockDb.select.mockImplementation(() => ({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue([mockSubscription]), + }), + })); + + const result = await service.listSubscriptions('prop-1'); + + expect((result[0] as any).secret).toBeUndefined(); + }); }); describe('deleteSubscription', () => { diff --git a/apps/api/src/modules/connect/connect-events.service.ts b/apps/api/src/modules/connect/connect-events.service.ts index 9bc401f..47c0cfd 100644 --- a/apps/api/src/modules/connect/connect-events.service.ts +++ b/apps/api/src/modules/connect/connect-events.service.ts @@ -13,6 +13,17 @@ export class ConnectEventsService { @Optional() private readonly deliveryService?: WebhookDeliveryService, ) {} + /** + * Strip the HMAC `secret` before a subscription row leaves the service. + * The secret is used server-side to sign webhook deliveries; returning it in + * API responses would let any caller forge signed deliveries to that + * subscriber. Callers supply the secret at creation and must retain their own copy. + */ + private redactSecret(row: T): Omit { + const { secret: _secret, ...rest } = row; + return rest; + } + /** * Create an event subscription for an OTAIP agent. */ @@ -29,14 +40,14 @@ export class ConnectEventsService { }) .returning(); - return subscription; + return this.redactSecret(subscription); } /** * List subscriptions for a property. */ async listSubscriptions(propertyId: string) { - return this.db + const rows = await this.db .select() .from(agentWebhookSubscriptions) .where( @@ -45,6 +56,7 @@ export class ConnectEventsService { eq(agentWebhookSubscriptions.isActive, true), ), ); + return rows.map((row: any) => this.redactSecret(row)); } /** From 4acc8f7161ccfbef6dc0bbb29113b474da766792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Mili=C4=87evi=C4=87?= Date: Fri, 19 Jun 2026 13:17:39 -0500 Subject: [PATCH 2/5] fix(security): SSRF guard on channel adapter endpoints + no redirect-follow - assertSafeChannelEndpoint(): block private/internal/metadata hosts for tenant-supplied channel baseUrl in production (opt-out CHANNEL_ALLOW_PRIVATE_ENDPOINTS for local mocks) - wire into booking-com / expedia / siteminder adapter fetch paths - redirect:'manual' on adapter + webhook-delivery fetches so a 302 to an internal host cannot bypass the pre-flight URL validation Co-Authored-By: Claude Opus 4.8 --- .../api/src/common/security/url-guard.spec.ts | 43 ++++++++++++++++++- apps/api/src/common/security/url-guard.ts | 16 +++++++ .../booking-com/booking-com.adapter.ts | 5 +++ .../adapters/expedia/expedia.adapter.ts | 4 +- .../adapters/siteminder/siteminder.adapter.ts | 3 ++ .../webhook/webhook-delivery.service.ts | 4 ++ 6 files changed, 72 insertions(+), 3 deletions(-) diff --git a/apps/api/src/common/security/url-guard.spec.ts b/apps/api/src/common/security/url-guard.spec.ts index 027ff74..bf2113a 100644 --- a/apps/api/src/common/security/url-guard.spec.ts +++ b/apps/api/src/common/security/url-guard.spec.ts @@ -1,5 +1,11 @@ -import { describe, it, expect } from 'vitest'; -import { isPrivateIp, isLiterallySafeHttpUrl, assertSafeOutboundUrl, UnsafeUrlError } from './url-guard'; +import { describe, it, expect, afterEach } from 'vitest'; +import { + isPrivateIp, + isLiterallySafeHttpUrl, + assertSafeOutboundUrl, + assertSafeChannelEndpoint, + UnsafeUrlError, +} from './url-guard'; describe('isPrivateIp', () => { it.each([ @@ -50,3 +56,36 @@ describe('assertSafeOutboundUrl', () => { await expect(assertSafeOutboundUrl('https://1.1.1.1/', { requireHttps: true })).resolves.toBeUndefined(); }); }); + +describe('assertSafeChannelEndpoint', () => { + const prevEnv = process.env['NODE_ENV']; + const prevAllow = process.env['CHANNEL_ALLOW_PRIVATE_ENDPOINTS']; + afterEach(() => { + process.env['NODE_ENV'] = prevEnv; + if (prevAllow === undefined) delete process.env['CHANNEL_ALLOW_PRIVATE_ENDPOINTS']; + else process.env['CHANNEL_ALLOW_PRIVATE_ENDPOINTS'] = prevAllow; + }); + + it('blocks an internal/metadata host in production', async () => { + process.env['NODE_ENV'] = 'production'; + delete process.env['CHANNEL_ALLOW_PRIVATE_ENDPOINTS']; + await expect(assertSafeChannelEndpoint('http://169.254.169.254/latest/meta-data/')).rejects.toBeInstanceOf(UnsafeUrlError); + }); + + it('allows a public host in production', async () => { + process.env['NODE_ENV'] = 'production'; + delete process.env['CHANNEL_ALLOW_PRIVATE_ENDPOINTS']; + await expect(assertSafeChannelEndpoint('https://1.1.1.1/ota')).resolves.toBeUndefined(); + }); + + it('allows private hosts outside production (local mock OTA servers)', async () => { + process.env['NODE_ENV'] = 'test'; + await expect(assertSafeChannelEndpoint('http://127.0.0.1:8080/ota')).resolves.toBeUndefined(); + }); + + it('allows private hosts in production when explicitly opted in', async () => { + process.env['NODE_ENV'] = 'production'; + process.env['CHANNEL_ALLOW_PRIVATE_ENDPOINTS'] = 'true'; + await expect(assertSafeChannelEndpoint('http://10.0.0.5/ota')).resolves.toBeUndefined(); + }); +}); diff --git a/apps/api/src/common/security/url-guard.ts b/apps/api/src/common/security/url-guard.ts index d5d93dd..a091d7e 100644 --- a/apps/api/src/common/security/url-guard.ts +++ b/apps/api/src/common/security/url-guard.ts @@ -88,6 +88,22 @@ export async function assertSafeOutboundUrl( } } +/** + * SSRF guard for outbound OTA channel-adapter requests. The endpoint base URL + * comes from tenant-supplied channel-connection config, so a property admin could + * point it at an internal/metadata host and trigger a server-side fetch. Block + * private targets in production. Local/dev (docker mock OTA servers on private + * hosts) is allowed unless explicitly locked down, mirroring the project's + * NODE_ENV / opt-in posture (cf. HAIP_ALLOW_INSECURE). + */ +export async function assertSafeChannelEndpoint(raw: string): Promise { + const enforce = + process.env['NODE_ENV'] === 'production' && + process.env['CHANNEL_ALLOW_PRIVATE_ENDPOINTS'] !== 'true'; + if (!enforce) return; + await assertSafeOutboundUrl(raw); +} + /** Sync, literal-only check for DTO validation (no DNS). */ export function isLiterallySafeHttpUrl(raw: string, opts: { requireHttps?: boolean } = {}): boolean { let url: URL; diff --git a/apps/api/src/modules/channel/adapters/booking-com/booking-com.adapter.ts b/apps/api/src/modules/channel/adapters/booking-com/booking-com.adapter.ts index e2c2619..477e7b4 100644 --- a/apps/api/src/modules/channel/adapters/booking-com/booking-com.adapter.ts +++ b/apps/api/src/modules/channel/adapters/booking-com/booking-com.adapter.ts @@ -15,6 +15,7 @@ import type { import type { BookingComConfig } from './booking-com.config'; import { DEFAULT_BOOKING_COM_CONFIG } from './booking-com.config'; import { buildOtaXml, parseOtaXml } from './booking-com.xml'; +import { assertSafeChannelEndpoint } from '../../../../common/security/url-guard'; import { mapAvailabilityToOta, mapRatesToOta, @@ -158,6 +159,7 @@ export class BookingComAdapter implements ChannelAdapter { body: unknown, ): Promise<{ ok: boolean; status: number; error?: string; data?: unknown }> { const url = `${config.baseUrl.replace(/\/$/, '')}${path}`; + await assertSafeChannelEndpoint(url); // SSRF: baseUrl is tenant-supplied const auth = Buffer.from(`${config.username}:${config.password}`).toString('base64'); const timeoutMs = config.timeoutMs ?? 30_000; const maxRetries = config.maxRetries ?? 3; @@ -171,6 +173,7 @@ export class BookingComAdapter implements ChannelAdapter { method, headers: { 'Content-Type': 'application/json', Authorization: `Basic ${auth}` }, body: JSON.stringify(body), + redirect: 'manual', // don't follow a redirect to an internal host (SSRF) signal: controller.signal, }); clearTimeout(timer); @@ -307,6 +310,7 @@ export class BookingComAdapter implements ChannelAdapter { xml: string, ): Promise> { const url = `${config.baseUrl}/${endpoint}`; + await assertSafeChannelEndpoint(url); // SSRF: baseUrl is tenant-supplied const auth = Buffer.from(`${config.username}:${config.password}`).toString('base64'); const timeoutMs = config.timeoutMs ?? 30_000; const maxRetries = config.maxRetries ?? 3; @@ -325,6 +329,7 @@ export class BookingComAdapter implements ChannelAdapter { Authorization: `Basic ${auth}`, }, body: xml, + redirect: 'manual', // don't follow a redirect to an internal host (SSRF) signal: controller.signal, }); diff --git a/apps/api/src/modules/channel/adapters/expedia/expedia.adapter.ts b/apps/api/src/modules/channel/adapters/expedia/expedia.adapter.ts index 01d71eb..bfb2c11 100644 --- a/apps/api/src/modules/channel/adapters/expedia/expedia.adapter.ts +++ b/apps/api/src/modules/channel/adapters/expedia/expedia.adapter.ts @@ -18,6 +18,7 @@ import { EXPEDIA_AR_NS, EXPEDIA_BC_NS, } from './expedia.config'; +import { assertSafeChannelEndpoint } from '../../../../common/security/url-guard'; import { buildExpediaXml, parseExpediaResponse } from './expedia.xml'; import { mapAvailabilityToExpedia, @@ -182,6 +183,7 @@ export class ExpediaAdapter implements ChannelAdapter { url: string, init: RequestInit, ): Promise<{ ok: boolean; status: number; text: string; error?: string }> { + await assertSafeChannelEndpoint(url); // SSRF: baseUrl is tenant-supplied const timeoutMs = config.timeoutMs ?? 30_000; const maxRetries = config.maxRetries ?? 3; let lastError: Error | null = null; @@ -189,7 +191,7 @@ export class ExpediaAdapter implements ChannelAdapter { try { const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), timeoutMs); - const res = await fetch(url, { ...init, signal: controller.signal }); + const res = await fetch(url, { ...init, redirect: 'manual', signal: controller.signal }); clearTimeout(timer); const text = await res.text(); if (!res.ok) { diff --git a/apps/api/src/modules/channel/adapters/siteminder/siteminder.adapter.ts b/apps/api/src/modules/channel/adapters/siteminder/siteminder.adapter.ts index a7e5537..c828728 100644 --- a/apps/api/src/modules/channel/adapters/siteminder/siteminder.adapter.ts +++ b/apps/api/src/modules/channel/adapters/siteminder/siteminder.adapter.ts @@ -15,6 +15,7 @@ import type { import type { SiteMinderConfig } from './siteminder.config'; import { DEFAULT_SITEMINDER_CONFIG } from './siteminder.config'; import { buildSoapEnvelope, parseSoapResponse } from './siteminder.soap'; +import { assertSafeChannelEndpoint } from '../../../../common/security/url-guard'; import { mapAvailabilityToOta, mapRatesToOta, @@ -261,6 +262,7 @@ export class SiteMinderAdapter implements ChannelAdapter { soapAction: string, ): Promise> { const url = config.baseUrl; + await assertSafeChannelEndpoint(url); // SSRF: baseUrl is tenant-supplied const timeoutMs = config.timeoutMs ?? 30_000; const maxRetries = config.maxRetries ?? 3; @@ -278,6 +280,7 @@ export class SiteMinderAdapter implements ChannelAdapter { SOAPAction: soapAction, }, body: soapXml, + redirect: 'manual', // don't follow a redirect to an internal host (SSRF) signal: controller.signal, }); diff --git a/apps/api/src/modules/webhook/webhook-delivery.service.ts b/apps/api/src/modules/webhook/webhook-delivery.service.ts index 0e5bdd0..95a5bdf 100644 --- a/apps/api/src/modules/webhook/webhook-delivery.service.ts +++ b/apps/api/src/modules/webhook/webhook-delivery.service.ts @@ -147,6 +147,10 @@ export class WebhookDeliveryService implements OnModuleInit, OnModuleDestroy { 'X-HAIP-Event-Type': delivery.eventType, }, body, + // The pre-flight assertSafeOutboundUrl only validates the FIRST URL. + // Without this, a 302 to http://169.254.169.254/… would be followed to + // an internal host, defeating the SSRF guard. + redirect: 'manual', signal: controller.signal, }); statusCode = resp.status; From f9174526100c71e841c40249ca48f17a2bb685a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Mili=C4=87evi=C4=87?= Date: Fri, 19 Jun 2026 13:19:56 -0500 Subject: [PATCH 3/5] fix(security): gate reports behind reports.view; strong OTA confirmation numbers - reports.controller: @RequirePermissions('reports.view') so financial/occupancy reports aren't readable by any authenticated property user (e.g. housekeeping) - OTA inbound generateConfirmationNumber: reuse the 128-bit CSPRNG token instead of Date.now()+Math.random() (was enumerable; confirmation # is a bearer credential) Co-Authored-By: Claude Opus 4.8 --- .../channel/inbound-reservation.service.spec.ts | 11 +++++++++++ .../modules/channel/inbound-reservation.service.ts | 9 +++++---- .../src/modules/reports/reports.controller.spec.ts | 13 +++++++++++++ apps/api/src/modules/reports/reports.controller.ts | 5 +++++ 4 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 apps/api/src/modules/reports/reports.controller.spec.ts diff --git a/apps/api/src/modules/channel/inbound-reservation.service.spec.ts b/apps/api/src/modules/channel/inbound-reservation.service.spec.ts index 4de443f..119adcd 100644 --- a/apps/api/src/modules/channel/inbound-reservation.service.spec.ts +++ b/apps/api/src/modules/channel/inbound-reservation.service.spec.ts @@ -372,4 +372,15 @@ describe('InboundReservationService', () => { expect(result.guestId).toBe('guest-1'); }); }); + + describe('confirmation number entropy', () => { + it('generates a high-entropy, non-time-derived confirmation number', () => { + const gen = () => (service as any).generateConfirmationNumber() as string; + const n = gen(); + // 128-bit Crockford token (CH- + 32 chars), not `CH--<4 random>`. + expect(n).toMatch(/^CH-[0-9A-Z]{32}$/); + const many = new Set(Array.from({ length: 200 }, gen)); + expect(many.size).toBe(200); + }); + }); }); diff --git a/apps/api/src/modules/channel/inbound-reservation.service.ts b/apps/api/src/modules/channel/inbound-reservation.service.ts index 08a1bf0..039cd39 100644 --- a/apps/api/src/modules/channel/inbound-reservation.service.ts +++ b/apps/api/src/modules/channel/inbound-reservation.service.ts @@ -6,6 +6,7 @@ import { ChannelService } from './channel.service'; import { ChannelAdapterFactory } from './channel-adapter.factory'; import { AriService } from './ari.service'; import { WebhookService } from '../webhook/webhook.service'; +import { generateConfirmationToken } from '../connect/connect-booking.service'; import type { ChannelReservation } from './channel-adapter.interface'; @Injectable() @@ -485,9 +486,9 @@ export class InboundReservationService { } private generateConfirmationNumber(): string { - const prefix = 'CH'; - const timestamp = Date.now().toString(36).toUpperCase(); - const random = Math.random().toString(36).substring(2, 6).toUpperCase(); - return `${prefix}-${timestamp}-${random}`; + // The confirmation number is a bearer credential for the booking, so it must + // be unguessable. The previous `Date.now()`-based value with 4 Math.random + // chars was enumerable; reuse the shared 128-bit CSPRNG token (Crockford b32). + return `CH-${generateConfirmationToken()}`; } } diff --git a/apps/api/src/modules/reports/reports.controller.spec.ts b/apps/api/src/modules/reports/reports.controller.spec.ts new file mode 100644 index 0000000..f57c09a --- /dev/null +++ b/apps/api/src/modules/reports/reports.controller.spec.ts @@ -0,0 +1,13 @@ +import { describe, it, expect } from 'vitest'; +import 'reflect-metadata'; +import { ReportsController } from './reports.controller'; +import { PERMISSIONS_KEY } from '../auth/permissions.decorator'; + +describe('ReportsController authorization', () => { + it('requires the reports.view permission on the controller', () => { + // Financial/occupancy reports must not be readable by any authenticated user + // (e.g. housekeeping) — PermissionsGuard enforces this metadata. + const perms = Reflect.getMetadata(PERMISSIONS_KEY, ReportsController); + expect(perms).toContain('reports.view'); + }); +}); diff --git a/apps/api/src/modules/reports/reports.controller.ts b/apps/api/src/modules/reports/reports.controller.ts index 1229efd..d023969 100644 --- a/apps/api/src/modules/reports/reports.controller.ts +++ b/apps/api/src/modules/reports/reports.controller.ts @@ -6,9 +6,14 @@ import { } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiQuery } from '@nestjs/swagger'; import { ReportsService } from './reports.service'; +import { RequirePermissions } from '../auth/permissions.decorator'; @ApiTags('reports') @Controller('reports') +// Financial / occupancy reports are management data. Without this, RolesGuard and +// PermissionsGuard default-allow any authenticated user (e.g. housekeeping) for a +// property they belong to. PermissionsGuard enforces this for every route below. +@RequirePermissions('reports.view') export class ReportsController { constructor(private readonly reportsService: ReportsService) {} From 4ae110f1f6b1bed40180ff27d74cea866d5f6edd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Mili=C4=87evi=C4=87?= Date: Fri, 19 Jun 2026 13:24:42 -0500 Subject: [PATCH 4/5] fix(security): require caller auth on the ChatGPT gateway (CRITICAL) The gateway injects HAIP's privileged platform Connect key on every upstream call, so an unauthenticated public gateway is an open door to cross-tenant Connect operations. Add an onRequest hook that requires a caller credential (Authorization: Bearer or x-api-key) on all action routes; /health, /openapi.json, /privacy and the landing page stay public. Fail-closed: if no GATEWAY_API_KEY is configured and GATEWAY_ALLOW_PUBLIC!=true, action routes 401. Timing-safe comparison. Wired in server.ts + api/index.ts; documented in .env.example. Co-Authored-By: Claude Opus 4.8 --- tools/haip-connect-gpt/.env.example | 10 +++ tools/haip-connect-gpt/api/index.ts | 2 + tools/haip-connect-gpt/src/app.auth.test.ts | 77 +++++++++++++++++++++ tools/haip-connect-gpt/src/app.ts | 47 +++++++++++++ tools/haip-connect-gpt/src/server.ts | 7 +- 5 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 tools/haip-connect-gpt/src/app.auth.test.ts diff --git a/tools/haip-connect-gpt/.env.example b/tools/haip-connect-gpt/.env.example index 96ef87f..121bc70 100644 --- a/tools/haip-connect-gpt/.env.example +++ b/tools/haip-connect-gpt/.env.example @@ -13,6 +13,16 @@ PORT=8080 # deployment domain if left unset; set it to your production domain for a stable spec. PUBLIC_BASE_URL=http://localhost:8080 +# --- Caller authentication (REQUIRED unless GATEWAY_ALLOW_PUBLIC=true) --- +# Credential the ChatGPT Action must present (Authorization: Bearer , or +# x-api-key). The gateway holds HAIP's privileged Connect key, so leaving the +# gateway open lets anyone who finds the URL drive the Connect API. Set this here +# AND in the ChatGPT Action's Authentication (API Key / Bearer) config. +GATEWAY_API_KEY= +# Explicit opt-out to keep the action routes public (the unauthenticated demo). +# Leave unset/false in any real deployment. +GATEWAY_ALLOW_PUBLIC=false + # --- Tool-call logging (Supabase project: haip-demo, direct Postgres) --- # Connection string for the haip_tool_calls table. Leave blank to disable # logging (gateway still serves normally). diff --git a/tools/haip-connect-gpt/api/index.ts b/tools/haip-connect-gpt/api/index.ts index 0c44f49..db7d0f0 100644 --- a/tools/haip-connect-gpt/api/index.ts +++ b/tools/haip-connect-gpt/api/index.ts @@ -33,6 +33,8 @@ const publicBaseUrl = const app = buildApp({ adapter: new HaipConnectAdapter({ baseUrl, apiKey }), publicBaseUrl, + gatewayApiKey: process.env['GATEWAY_API_KEY'], + allowPublic: process.env['GATEWAY_ALLOW_PUBLIC'] === 'true', }); const ready = app.ready(); diff --git a/tools/haip-connect-gpt/src/app.auth.test.ts b/tools/haip-connect-gpt/src/app.auth.test.ts new file mode 100644 index 0000000..f3bb491 --- /dev/null +++ b/tools/haip-connect-gpt/src/app.auth.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect } from 'vitest'; +import { buildApp } from './app.js'; + +function stubAdapter() { + return { + searchHotels: async () => ({ results: [] }), + getProperty: async () => ({ id: 'p1' }), + createReservation: async () => ({ confirmationNumber: 'HAIP-X' }), + getReservation: async () => ({ confirmationNumber: 'HAIP-X' }), + modifyReservation: async () => ({ ok: true }), + cancelReservation: async () => ({ ok: true }), + upstreamHealthy: async () => true, + } as any; +} + +const GATEWAY_KEY = 'gw_secret_key'; + +describe('gateway authentication', () => { + it('allows /health without a credential', async () => { + const app = buildApp({ adapter: stubAdapter(), publicBaseUrl: 'http://x', gatewayApiKey: GATEWAY_KEY }); + const res = await app.inject({ method: 'GET', url: '/health' }); + expect(res.statusCode).toBe(200); + await app.close(); + }); + + it('allows /openapi.json without a credential', async () => { + const app = buildApp({ adapter: stubAdapter(), publicBaseUrl: 'http://x', gatewayApiKey: GATEWAY_KEY }); + const res = await app.inject({ method: 'GET', url: '/openapi.json' }); + expect(res.statusCode).toBe(200); + await app.close(); + }); + + it('rejects an action route with no credential (401)', async () => { + const app = buildApp({ adapter: stubAdapter(), publicBaseUrl: 'http://x', gatewayApiKey: GATEWAY_KEY }); + const res = await app.inject({ method: 'POST', url: '/hotels/search', payload: {} }); + expect(res.statusCode).toBe(401); + await app.close(); + }); + + it('rejects an action route with a wrong credential (401)', async () => { + const app = buildApp({ adapter: stubAdapter(), publicBaseUrl: 'http://x', gatewayApiKey: GATEWAY_KEY }); + const res = await app.inject({ + method: 'POST', + url: '/hotels/search', + headers: { authorization: 'Bearer wrong' }, + payload: {}, + }); + expect(res.statusCode).toBe(401); + await app.close(); + }); + + it('allows an action route with the correct Bearer credential', async () => { + const app = buildApp({ adapter: stubAdapter(), publicBaseUrl: 'http://x', gatewayApiKey: GATEWAY_KEY }); + const res = await app.inject({ + method: 'POST', + url: '/hotels/search', + headers: { authorization: `Bearer ${GATEWAY_KEY}` }, + payload: {}, + }); + expect(res.statusCode).toBe(200); + await app.close(); + }); + + it('fails closed: no key configured and not explicitly public → 401', async () => { + const app = buildApp({ adapter: stubAdapter(), publicBaseUrl: 'http://x' }); + const res = await app.inject({ method: 'POST', url: '/hotels/search', payload: {} }); + expect(res.statusCode).toBe(401); + await app.close(); + }); + + it('explicit opt-out (allowPublic) keeps the demo open', async () => { + const app = buildApp({ adapter: stubAdapter(), publicBaseUrl: 'http://x', allowPublic: true }); + const res = await app.inject({ method: 'POST', url: '/hotels/search', payload: {} }); + expect(res.statusCode).toBe(200); + await app.close(); + }); +}); diff --git a/tools/haip-connect-gpt/src/app.ts b/tools/haip-connect-gpt/src/app.ts index 26bbcc8..4771895 100644 --- a/tools/haip-connect-gpt/src/app.ts +++ b/tools/haip-connect-gpt/src/app.ts @@ -8,6 +8,7 @@ import Fastify, { type FastifyInstance, type FastifyReply, type FastifyRequest } from 'fastify'; import cors from '@fastify/cors'; +import { timingSafeEqual } from 'node:crypto'; import { HaipConnectAdapter, UpstreamError, @@ -23,6 +24,38 @@ import { indexHtml, privacyHtml } from './pages.js'; export interface AppOptions { adapter: HaipConnectAdapter; publicBaseUrl: string; + /** + * Credential the caller (the ChatGPT Action) must present to reach the action + * routes — sent as `Authorization: Bearer ` or `x-api-key`. The gateway + * holds HAIP's privileged upstream Connect key, so without this anyone on the + * internet who finds the URL can drive the Connect API. Configure via GATEWAY_API_KEY. + */ + gatewayApiKey?: string; + /** + * Explicit opt-out that leaves the action routes public (the unauthenticated + * demo). Mirrors HAIP_ALLOW_INSECURE — secure-by-default otherwise. + */ + allowPublic?: boolean; +} + +/** Public routes that never require a caller credential. */ +const PUBLIC_PATHS = new Set(['/health', '/openapi.json', '/', '/privacy']); + +function timingSafeEqualStr(a: string, b: string): boolean { + const ab = Buffer.from(a); + const bb = Buffer.from(b); + if (ab.length !== bb.length) return false; + return timingSafeEqual(ab, bb); +} + +/** Extract the caller credential from `Authorization: Bearer` or `x-api-key`. */ +function extractCredential(req: FastifyRequest): string | null { + const auth = req.headers['authorization']; + if (typeof auth === 'string' && auth.startsWith('Bearer ')) return auth.slice(7).trim(); + const key = req.headers['x-api-key']; + if (typeof key === 'string') return key; + if (Array.isArray(key)) return key[0] ?? null; + return null; } export function buildApp(opts: AppOptions): FastifyInstance { @@ -31,6 +64,20 @@ export function buildApp(opts: AppOptions): FastifyInstance { app.register(cors, { origin: true }); + // Require a caller credential on every non-public route. The gateway proxies + // requests with HAIP's privileged upstream `x-api-key`, so an unauthenticated + // gateway is an open door to the Connect API. Fail-closed: if no key is + // configured and `allowPublic` was not explicitly set, refuse action routes. + app.addHook('onRequest', async (req, reply) => { + const path = (req.url.split('?')[0] ?? req.url).replace(/\/+$/, '') || '/'; + if (PUBLIC_PATHS.has(path)) return; + if (opts.allowPublic) return; + const provided = extractCredential(req); + if (!opts.gatewayApiKey || !provided || !timingSafeEqualStr(provided, opts.gatewayApiKey)) { + reply.code(401).send({ error: 'unauthorized', message: 'A valid gateway credential is required.' }); + } + }); + app.get('/health', async () => ({ status: 'ok', service: 'haip-connect-gpt', diff --git a/tools/haip-connect-gpt/src/server.ts b/tools/haip-connect-gpt/src/server.ts index c670748..bdd1925 100644 --- a/tools/haip-connect-gpt/src/server.ts +++ b/tools/haip-connect-gpt/src/server.ts @@ -23,7 +23,12 @@ const port = Number(process.env['PORT'] ?? 8080); const publicBaseUrl = process.env['PUBLIC_BASE_URL'] ?? `http://localhost:${port}`; const adapter = new HaipConnectAdapter({ baseUrl, apiKey }); -const app = buildApp({ adapter, publicBaseUrl }); +const app = buildApp({ + adapter, + publicBaseUrl, + gatewayApiKey: process.env['GATEWAY_API_KEY'], + allowPublic: process.env['GATEWAY_ALLOW_PUBLIC'] === 'true', +}); app .listen({ port, host: '0.0.0.0' }) From 334f8487bd2ffa51f626b3efd2bf7da02dd2b200 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Mili=C4=87evi=C4=87?= Date: Fri, 19 Jun 2026 13:29:16 -0500 Subject: [PATCH 5/5] fix(security): SMS per-property quota, staging config gate, gate Swagger in prod, demo warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - notifications: in-memory per-property SMS quota (toll-fraud/spam guard on the free-form, billable SMS endpoint); tunable via SMS_RATE_LIMIT_MAX/WINDOW_MS - assert-secure-config: enforce for 'staging' too (not just 'production') so a staging host can't silently boot with AUTH_ENABLED=false - main.ts: only mount Swagger /docs outside production (opt back in with SWAGGER_ENABLED=true) - render.yaml: loud DEMO-ONLY warning — auth-off blueprint is not a production template Co-Authored-By: Claude Opus 4.8 --- .../config/assert-secure-config.spec.ts | 4 +++ .../src/common/config/assert-secure-config.ts | 7 ++++- apps/api/src/main.ts | 12 ++++++-- .../notification.service.spec.ts | 29 ++++++++++++++++++- .../notifications/notification.service.ts | 24 ++++++++++++++- render.yaml | 10 +++++++ 6 files changed, 80 insertions(+), 6 deletions(-) diff --git a/apps/api/src/common/config/assert-secure-config.spec.ts b/apps/api/src/common/config/assert-secure-config.spec.ts index 588a6f7..3b6a938 100644 --- a/apps/api/src/common/config/assert-secure-config.spec.ts +++ b/apps/api/src/common/config/assert-secure-config.spec.ts @@ -14,6 +14,10 @@ describe('assertSecureConfig', () => { expect(() => assertSecureConfig({ NODE_ENV: 'production', AUTH_ENABLED: 'true', STRIPE_MODE: 'mock' } as any)).toThrow(/STRIPE_MODE=mock/); }); + it('also refuses to boot in staging with AUTH disabled (not just production)', () => { + expect(() => assertSecureConfig({ NODE_ENV: 'staging', AUTH_ENABLED: 'false', STRIPE_MODE: 'live' } as any)).toThrow(/AUTH_ENABLED=false/); + }); + it('allows the explicit insecure opt-in (public demo)', () => { expect(() => assertSecureConfig({ NODE_ENV: 'production', AUTH_ENABLED: 'false', STRIPE_MODE: 'mock', HAIP_ALLOW_INSECURE: 'true' } as any), diff --git a/apps/api/src/common/config/assert-secure-config.ts b/apps/api/src/common/config/assert-secure-config.ts index ecec122..a934aa9 100644 --- a/apps/api/src/common/config/assert-secure-config.ts +++ b/apps/api/src/common/config/assert-secure-config.ts @@ -8,7 +8,12 @@ * process.env at startup. */ export function assertSecureConfig(env: NodeJS.ProcessEnv = process.env): void { - if (env['NODE_ENV'] !== 'production') return; + // Enforce for any production-like environment, not just NODE_ENV=production — + // a host run as 'staging' must not silently boot with auth off either. Local + // dev/test ('development', 'test', or unset) stays permissive. + const nodeEnv = env['NODE_ENV']; + const productionLike = nodeEnv === 'production' || nodeEnv === 'staging'; + if (!productionLike) return; if (env['HAIP_ALLOW_INSECURE'] === 'true') return; const problems: string[] = []; if (env['AUTH_ENABLED'] === 'false') problems.push('AUTH_ENABLED=false'); diff --git a/apps/api/src/main.ts b/apps/api/src/main.ts index daa2c80..f2dc601 100644 --- a/apps/api/src/main.ts +++ b/apps/api/src/main.ts @@ -74,14 +74,20 @@ async function bootstrap() { .addTag('health', 'System health checks') .build(); - const document = SwaggerModule.createDocument(app, config); - SwaggerModule.setup('docs', app, document); + // Expose the Swagger UI everywhere except production (it maps the full API + // surface for an attacker). The public demo can opt back in with SWAGGER_ENABLED=true. + const serveDocs = + process.env['NODE_ENV'] !== 'production' || process.env['SWAGGER_ENABLED'] === 'true'; + if (serveDocs) { + const document = SwaggerModule.createDocument(app, config); + SwaggerModule.setup('docs', app, document); + } const port = process.env['PORT'] ?? 3000; await app.listen(port); console.log(`HAIP API running on http://localhost:${port}`); - console.log(`OpenAPI docs at http://localhost:${port}/docs`); + if (serveDocs) console.log(`OpenAPI docs at http://localhost:${port}/docs`); } bootstrap(); diff --git a/apps/api/src/modules/notifications/notification.service.spec.ts b/apps/api/src/modules/notifications/notification.service.spec.ts index 85978ec..eb28c69 100644 --- a/apps/api/src/modules/notifications/notification.service.spec.ts +++ b/apps/api/src/modules/notifications/notification.service.spec.ts @@ -1,4 +1,5 @@ -import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { HttpException } from '@nestjs/common'; import { NotificationService } from './notification.service'; import { ConsoleSmsProvider } from './providers/console-sms.provider'; import { TwilioSmsProvider } from './providers/twilio-sms.provider'; @@ -51,4 +52,30 @@ describe('NotificationService', () => { expect(data).toMatchObject({ channel: 'sms', provider: 'console', success: false }); expect(propertyId).toBe(PROPERTY_ID); }); + + describe('per-property SMS quota (toll-fraud / spam guard)', () => { + const prev = process.env['SMS_RATE_LIMIT_MAX']; + beforeEach(() => { process.env['SMS_RATE_LIMIT_MAX'] = '2'; }); + afterEach(() => { + if (prev === undefined) delete process.env['SMS_RATE_LIMIT_MAX']; + else process.env['SMS_RATE_LIMIT_MAX'] = prev; + }); + + it('throttles SMS for a property beyond the configured limit', async () => { + const twilio = { isConfigured: () => false } as unknown as TwilioSmsProvider; + const service = new NotificationService(twilio, consoleProvider, webhooks as any); + await service.sendSms(PROPERTY_ID, '+15551230000', 'hi'); + await service.sendSms(PROPERTY_ID, '+15551230000', 'hi'); + await expect(service.sendSms(PROPERTY_ID, '+15551230000', 'hi')).rejects.toBeInstanceOf(HttpException); + }); + + it('counts the quota independently per property', async () => { + const twilio = { isConfigured: () => false } as unknown as TwilioSmsProvider; + const service = new NotificationService(twilio, consoleProvider, webhooks as any); + await service.sendSms(PROPERTY_ID, '+15551230000', 'hi'); + await service.sendSms(PROPERTY_ID, '+15551230000', 'hi'); + // A different property still has quota. + await expect(service.sendSms('22222222-2222-2222-2222-222222222222', '+15551230000', 'hi')).resolves.toBeDefined(); + }); + }); }); diff --git a/apps/api/src/modules/notifications/notification.service.ts b/apps/api/src/modules/notifications/notification.service.ts index 3a2c6c5..dcfb4cc 100644 --- a/apps/api/src/modules/notifications/notification.service.ts +++ b/apps/api/src/modules/notifications/notification.service.ts @@ -1,4 +1,4 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { Injectable, Logger, HttpException, HttpStatus } from '@nestjs/common'; import { WebhookService } from '../webhook/webhook.service'; import { TwilioSmsProvider } from './providers/twilio-sms.provider'; import { ConsoleSmsProvider } from './providers/console-sms.provider'; @@ -29,12 +29,34 @@ export class NotificationService { return this.twilio.isConfigured() ? this.twilio : this.console; } + // In-memory per-property SMS quota (interim, single-instance). SMS is a billable + // outbound action with a free-form destination, so an authenticated front-desk + // user could otherwise drive the property's Twilio account to arbitrary/premium + // numbers (toll fraud / spam relay). Tunable via env. + private readonly smsHits = new Map(); + private assertSmsQuota(propertyId: string): void { + const max = Number(process.env['SMS_RATE_LIMIT_MAX'] ?? 60); + const windowMs = Number(process.env['SMS_RATE_LIMIT_WINDOW_MS'] ?? 3_600_000); + if (!Number.isFinite(max) || max <= 0) return; // disabled + const now = Date.now(); + const entry = this.smsHits.get(propertyId); + if (!entry || now >= entry.resetAt) { + this.smsHits.set(propertyId, { count: 1, resetAt: now + windowMs }); + return; + } + entry.count++; + if (entry.count > max) { + throw new HttpException('SMS rate limit exceeded for this property', HttpStatus.TOO_MANY_REQUESTS); + } + } + /** * Send an SMS to a guest and record the dispatch against the property. * Never throws on send failure — returns a structured result so callers * (e.g. the guest-comms agent) can decide whether to retry or escalate. */ async sendSms(propertyId: string, to: string, body: string): Promise { + this.assertSmsQuota(propertyId); const provider = this.smsProvider(); const result = await provider.send({ to, body }); diff --git a/render.yaml b/render.yaml index 14aaa20..cbeb6f6 100644 --- a/render.yaml +++ b/render.yaml @@ -1,5 +1,15 @@ # HAIP — Render Blueprint (one-click cloud demo) # +# ⚠️ DEMO ONLY — NOT A SECURE PRODUCTION TEMPLATE. ⚠️ +# This blueprint runs with AUTH_ENABLED=false + HAIP_ALLOW_INSECURE=true + mock +# payments so the public demo is browsable with zero setup. That means EVERY API +# endpoint (incl. /api/v1/connect/*) is reachable WITHOUT authentication. Do NOT +# point real guest/PII or live payments at a deployment using these values. +# For a real deployment: remove HAIP_ALLOW_INSECURE, set AUTH_ENABLED=true with a +# configured Keycloak, set STRIPE_MODE=live/test with real keys, and set a strong +# CONNECT_API_KEY. (The API refuses to boot production-like with auth off UNLESS +# HAIP_ALLOW_INSECURE=true — so removing that line is the intended hardening step.) +# # Deploy a fully-working HAIP demo (API + dashboard + AI agents, seeded) with no # local setup. Render provisions Postgres + Redis, builds the Docker image, runs # migrate+seed via preDeployCommand, then serves the app.