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/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/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/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/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/channel/inbound-reservation.service.spec.ts b/apps/api/src/modules/channel/inbound-reservation.service.spec.ts index 2ca1051..119adcd 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,41 @@ 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'); + }); + }); + + 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 a3ae35d..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() @@ -122,7 +123,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 +399,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 +434,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; @@ -495,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/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)); } /** 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/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) {} 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; 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. 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' })