diff --git a/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-mobile-push.test.ts b/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-mobile-push.test.ts index 16d62ded02d..0cdd8b58029 100644 --- a/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-mobile-push.test.ts +++ b/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-mobile-push.test.ts @@ -130,7 +130,8 @@ describe('sendMobilePush action', () => { it('should send notification for custom hostname', async () => { const externalId = defaultExternalId - const notifyReqUrl = `https://my-api.com/v1/Services/${pushServiceSid}/Notifications` + const customHostname = 'push.custom.twilio.com' + const notifyReqUrl = `https://${customHostname}/v1/Services/${pushServiceSid}/Notifications` const notifyReqBody = getDefaultExpectedNotifyApiReq(externalId) nock(`https://content.twilio.com`).get(`/v1/Content/${contentSid}`).reply(200, defaultTemplate) @@ -138,13 +139,22 @@ describe('sendMobilePush action', () => { const responses = await testAction({ settingsOverrides: { - twilioHostname: 'my-api.com' + twilioHostname: customHostname } }) expect(responses[1].url).toStrictEqual(notifyReqUrl) expect(responses[1].status).toEqual(201) expect(responses[1].data).toMatchObject(externalId) }) + + it('should reject a non-Twilio custom hostname without sending credentials', async () => { + nock(`https://content.twilio.com`).get(`/v1/Content/${contentSid}`).reply(200, defaultTemplate) + + // No nock interceptor for the notify request: it must never be made, so credentials are never forwarded. + await expect(testAction({ settingsOverrides: { twilioHostname: 'my-api.com' } })).rejects.toThrow( + /Invalid Twilio hostname/ + ) + }) }) describe('error handling', () => { diff --git a/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-sms.test.ts b/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-sms.test.ts index 09b9546169c..897034620d1 100644 --- a/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-sms.test.ts +++ b/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-sms.test.ts @@ -391,7 +391,7 @@ describe.each(['stage', 'production'])('%s environment', (environment) => { }) }) - const twilioHostname = 'api.nottwilio.com' + const twilioHostname = 'api.custom.twilio.com' const twilioRequest = nock(`https://${twilioHostname}/2010-04-01/Accounts/a`) .post('/Messages.json', expectedTwilioRequest.toString()) @@ -409,6 +409,20 @@ describe.each(['stage', 'production'])('%s environment', (environment) => { expect(twilioRequest.isDone()).toEqual(true) }) + it('should reject a non-Twilio custom hostname without sending credentials', async () => { + const twilioHostname = 'api.nottwilio.com' + + // No nock interceptor: the request must never be made, so credentials are never forwarded. + await expect( + testAction({ + settingsOverrides: { twilioHostname }, + mappingOverrides: { + externalIds: [{ type: 'phone', id: '+1 (505) 555-4555', subscriptionStatus: true, channelType: 'sms' }] + } + }) + ).rejects.toThrow(/Invalid Twilio hostname/) + }) + it('should send SMS with custom metadata', async () => { const expectedTwilioRequest = new URLSearchParams({ Body: 'Hello world, jane!', diff --git a/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-whatsapp.test.ts b/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-whatsapp.test.ts index 208e5f02eed..969cb059719 100644 --- a/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-whatsapp.test.ts +++ b/packages/destination-actions/src/destinations/engage-messaging-twilio/__tests__/send-whatsapp.test.ts @@ -249,7 +249,7 @@ describe.each(['stage', 'production'])('%s environment', (environment) => { Tags: defaultTags }) - const twilioHostname = 'api.nottwilio.com' + const twilioHostname = 'api.custom.twilio.com' const twilioRequest = nock(`https://${twilioHostname}/2010-04-01/Accounts/a`) .post('/Messages.json', expectedTwilioRequest.toString()) @@ -262,6 +262,13 @@ describe.each(['stage', 'production'])('%s environment', (environment) => { expect(twilioRequest.isDone()).toEqual(true) }) + it('should reject a non-Twilio custom hostname without sending credentials', async () => { + // No nock interceptor: the request must never be made, so credentials are never forwarded. + await expect(testAction({ settingsOverrides: { twilioHostname: 'api.nottwilio.com' } })).rejects.toThrow( + /Invalid Twilio hostname/ + ) + }) + it('should send WhatsApp with custom metadata', async () => { const expectedTwilioRequest = new URLSearchParams({ ContentSid: defaultTemplateSid, diff --git a/packages/destination-actions/src/destinations/engage-messaging-twilio/index.ts b/packages/destination-actions/src/destinations/engage-messaging-twilio/index.ts index 6ad877c4f44..78cdd9c92af 100644 --- a/packages/destination-actions/src/destinations/engage-messaging-twilio/index.ts +++ b/packages/destination-actions/src/destinations/engage-messaging-twilio/index.ts @@ -3,6 +3,7 @@ import type { Settings } from './generated-types' import { actionDefinition as sendSms } from './sendSms' import { actionDefinition as sendWhatsApp } from './sendWhatsApp' import { actionDefinition as sendMobilePush } from './sendMobilePush' +import { validateTwilioHostname } from './utils' const getRange = (val: number): { value: number; label: string }[] => { return Array(val) @@ -167,7 +168,7 @@ export const destinationDefinition: DestinationDefinition = { } }, testAuthentication: (request, options) => { - const hostName = options.settings.twilioHostname ?? 'api.twilio.com' + const hostName = validateTwilioHostname(options.settings.twilioHostname ?? 'api.twilio.com') return request(`https://${hostName}/2010-04-01`) } }, diff --git a/packages/destination-actions/src/destinations/engage-messaging-twilio/sendMobilePush/PushSender.ts b/packages/destination-actions/src/destinations/engage-messaging-twilio/sendMobilePush/PushSender.ts index 95b5bc3e2ea..2928961ecf7 100644 --- a/packages/destination-actions/src/destinations/engage-messaging-twilio/sendMobilePush/PushSender.ts +++ b/packages/destination-actions/src/destinations/engage-messaging-twilio/sendMobilePush/PushSender.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ /* eslint-disable @typescript-eslint/no-unsafe-call */ -import { TwilioMessageSender } from '../utils' +import { TwilioMessageSender, validateTwilioHostname } from '../utils' import { ExtId, track } from '@segment/actions-shared' import type { Payload as PushPayload } from './generated-types' import { ContentTemplateTypes } from '../utils/types' @@ -31,7 +31,9 @@ export class PushSender extends TwilioMessageSender { private DEFAULT_HOSTNAME = 'push.ashburn.us1.twilio.com' get twilioHostname() { - return this.settings.twilioHostname?.length ? this.settings.twilioHostname : this.DEFAULT_HOSTNAME + return validateTwilioHostname( + this.settings.twilioHostname?.length ? this.settings.twilioHostname : this.DEFAULT_HOSTNAME + ) } get twilioToken() { //TODO cache this (lazy load) diff --git a/packages/destination-actions/src/destinations/engage-messaging-twilio/utils/PhoneMessageSender.ts b/packages/destination-actions/src/destinations/engage-messaging-twilio/utils/PhoneMessageSender.ts index 6ad3dc31995..a5b9e4f641f 100644 --- a/packages/destination-actions/src/destinations/engage-messaging-twilio/utils/PhoneMessageSender.ts +++ b/packages/destination-actions/src/destinations/engage-messaging-twilio/utils/PhoneMessageSender.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/no-non-null-assertion */ /* eslint-disable @typescript-eslint/no-unsafe-call */ -import { TwilioMessageSender, TwilioPayloadBase } from './TwilioMessageSender' +import { TwilioMessageSender, TwilioPayloadBase, validateTwilioHostname } from './TwilioMessageSender' import { OperationDecorator, TrackedError, OperationContext, ExtId } from '@segment/actions-shared' /** @@ -14,7 +14,7 @@ export abstract class PhoneMessageSender ex abstract getBody(phone: string): Promise get twilioHostname() { - return this.settings.twilioHostname ?? this.DEFAULT_HOSTNAME + return validateTwilioHostname(this.settings.twilioHostname ?? this.DEFAULT_HOSTNAME) } get twilioToken() { return Buffer.from(`${this.settings.twilioApiKeySID}:${this.settings.twilioApiKeySecret}`).toString('base64') diff --git a/packages/destination-actions/src/destinations/engage-messaging-twilio/utils/TwilioMessageSender.ts b/packages/destination-actions/src/destinations/engage-messaging-twilio/utils/TwilioMessageSender.ts index 7f3a7ec18f6..b1948e2fd93 100644 --- a/packages/destination-actions/src/destinations/engage-messaging-twilio/utils/TwilioMessageSender.ts +++ b/packages/destination-actions/src/destinations/engage-messaging-twilio/utils/TwilioMessageSender.ts @@ -8,6 +8,24 @@ import { track, MessageSendPerformer, MessagePayloadBase } from '@segment/action const Liquid = new LiquidJs() +// Only hostnames within Twilio's domain may receive requests, which carry the +// account's Basic-auth API key. The `twilioHostname` setting is customer-editable, +// so without this allowlist an arbitrary host could be used to exfiltrate the +// credentials (SECOPS-25241). Disallowing '@', '/' and ':' also blocks userinfo +// and path tricks such as "api.twilio.com@attacker.com". +const TWILIO_HOSTNAME_REGEX = /^([a-z0-9-]+\.)+twilio\.com$/ + +export function validateTwilioHostname(hostname: string): string { + // Hostnames are case-insensitive, so normalize before matching to avoid + // rejecting valid hosts like "Api.Twilio.Com". + if (!TWILIO_HOSTNAME_REGEX.test(hostname.toLowerCase())) { + throw new PayloadValidationError( + `Invalid Twilio hostname: "${hostname}". Hostname must be within the twilio.com domain.` + ) + } + return hostname +} + export interface TwilioPayloadBase extends MessagePayloadBase { contentSid?: string }