diff --git a/apps/api/src/common/security/url-guard.spec.ts b/apps/api/src/common/security/url-guard.spec.ts index bf2113a..d66be1b 100644 --- a/apps/api/src/common/security/url-guard.spec.ts +++ b/apps/api/src/common/security/url-guard.spec.ts @@ -88,4 +88,10 @@ describe('assertSafeChannelEndpoint', () => { process.env['CHANNEL_ALLOW_PRIVATE_ENDPOINTS'] = 'true'; await expect(assertSafeChannelEndpoint('http://10.0.0.5/ota')).resolves.toBeUndefined(); }); + + it('also blocks internal hosts in staging (production-like)', async () => { + process.env['NODE_ENV'] = 'staging'; + delete process.env['CHANNEL_ALLOW_PRIVATE_ENDPOINTS']; + await expect(assertSafeChannelEndpoint('http://169.254.169.254/latest/meta-data/')).rejects.toBeInstanceOf(UnsafeUrlError); + }); }); diff --git a/apps/api/src/common/security/url-guard.ts b/apps/api/src/common/security/url-guard.ts index a091d7e..5b79fd1 100644 --- a/apps/api/src/common/security/url-guard.ts +++ b/apps/api/src/common/security/url-guard.ts @@ -97,9 +97,11 @@ export async function assertSafeOutboundUrl( * 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'; + // Enforce for any production-like environment (production OR staging), matching + // assertSecureConfig — staging is internet-adjacent and must not allow SSRF either. + const nodeEnv = process.env['NODE_ENV']; + const productionLike = nodeEnv === 'production' || nodeEnv === 'staging'; + const enforce = productionLike && process.env['CHANNEL_ALLOW_PRIVATE_ENDPOINTS'] !== 'true'; if (!enforce) return; await assertSafeOutboundUrl(raw); } diff --git a/apps/api/src/modules/notifications/notification.service.spec.ts b/apps/api/src/modules/notifications/notification.service.spec.ts index eb28c69..fc3fd3d 100644 --- a/apps/api/src/modules/notifications/notification.service.spec.ts +++ b/apps/api/src/modules/notifications/notification.service.spec.ts @@ -69,6 +69,26 @@ describe('NotificationService', () => { await expect(service.sendSms(PROPERTY_ID, '+15551230000', 'hi')).rejects.toBeInstanceOf(HttpException); }); + it('still enforces with a default limit when the env value is invalid (no fail-open)', async () => { + process.env['SMS_RATE_LIMIT_MAX'] = 'not-a-number'; + process.env['SMS_RATE_LIMIT_WINDOW_MS'] = 'garbage'; + const twilio = { isConfigured: () => false } as unknown as TwilioSmsProvider; + const service = new NotificationService(twilio, consoleProvider, webhooks as any); + // Default limit is 60 → the 61st send for one property must be throttled. + for (let i = 0; i < 60; i++) await service.sendSms(PROPERTY_ID, '+15551230000', 'hi'); + await expect(service.sendSms(PROPERTY_ID, '+15551230000', 'hi')).rejects.toBeInstanceOf(HttpException); + delete process.env['SMS_RATE_LIMIT_WINDOW_MS']; + }); + + it('treats an explicit 0 limit as disabled', async () => { + process.env['SMS_RATE_LIMIT_MAX'] = '0'; + const twilio = { isConfigured: () => false } as unknown as TwilioSmsProvider; + const service = new NotificationService(twilio, consoleProvider, webhooks as any); + for (let i = 0; i < 5; i++) { + await expect(service.sendSms(PROPERTY_ID, '+15551230000', 'hi')).resolves.toBeDefined(); + } + }); + 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); diff --git a/apps/api/src/modules/notifications/notification.service.ts b/apps/api/src/modules/notifications/notification.service.ts index dcfb4cc..22e2d65 100644 --- a/apps/api/src/modules/notifications/notification.service.ts +++ b/apps/api/src/modules/notifications/notification.service.ts @@ -35,9 +35,14 @@ export class NotificationService { // 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 + // Robust parsing: an INVALID env value falls back to the safe default (the + // limiter stays ON — no fail-open on config drift). Only an explicit, valid + // value <= 0 disables it. + const rawMax = Number(process.env['SMS_RATE_LIMIT_MAX']); + const max = Number.isFinite(rawMax) ? rawMax : 60; + if (max <= 0) return; // explicitly disabled by the operator + const rawWindow = Number(process.env['SMS_RATE_LIMIT_WINDOW_MS']); + const windowMs = Number.isFinite(rawWindow) && rawWindow > 0 ? rawWindow : 3_600_000; const now = Date.now(); const entry = this.smsHits.get(propertyId); if (!entry || now >= entry.resetAt) {