Skip to content

Commit 9a050cd

Browse files
sunitaprajapati89Sunita Prajapatiabueide
authored
fix(core): validate CDN-supplied apiHost before building upload URL (#1312)
Co-authored-by: Sunita Prajapati <sunita.kuamari@daffodilsw.com> Co-authored-by: Andrea Bueide <abueide@twilio.com>
1 parent 39ee343 commit 9a050cd

5 files changed

Lines changed: 84 additions & 5 deletions

File tree

devbox.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@
199199
},
200200
"nodejs@22": {
201201
"last_modified": "2026-03-27T11:17:38Z",
202-
"plugin_version": "0.0.4",
202+
"plugin_version": "0.0.2",
203203
"resolved": "github:NixOS/nixpkgs/832efc09b4caf6b4569fbf9dc01bec3082a00611#nodejs_22",
204204
"source": "devbox-search",
205205
"version": "22.22.2",

packages/core/src/__tests__/util.test.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { UserTraits } from '../types';
2-
import { chunk, allSettled, deepCompare, getURL } from '../util';
2+
import {
3+
chunk,
4+
allSettled,
5+
deepCompare,
6+
getURL,
7+
validateApiHost,
8+
} from '../util';
39

410
describe('#chunk', () => {
511
it('handles empty array', () => {
@@ -210,3 +216,39 @@ describe('getURL function', () => {
210216
);
211217
});
212218
});
219+
220+
describe('validateApiHost', () => {
221+
it('accepts a bare hostname', () => {
222+
expect(validateApiHost('api.segment.io')).toBe(true);
223+
});
224+
225+
it('accepts hostname with path (normal Segment format)', () => {
226+
expect(validateApiHost('api.segment.io/v1')).toBe(true);
227+
expect(validateApiHost('events.eu1.segmentapis.com')).toBe(true);
228+
});
229+
230+
it('accepts hostname with port', () => {
231+
expect(validateApiHost('api.segment.io:443/v1')).toBe(true);
232+
});
233+
234+
it('rejects values with a scheme', () => {
235+
expect(validateApiHost('https://api.segment.io/v1')).toBe(false);
236+
expect(validateApiHost('http://api.segment.io/v1')).toBe(false);
237+
});
238+
239+
it('rejects values with credentials', () => {
240+
expect(validateApiHost('user:pass@api.segment.io')).toBe(false);
241+
});
242+
243+
it('rejects values with a query string', () => {
244+
expect(validateApiHost('attacker.com/collect?x=')).toBe(false);
245+
});
246+
247+
it('rejects values with a fragment', () => {
248+
expect(validateApiHost('attacker.com/path#fragment')).toBe(false);
249+
});
250+
251+
it('rejects empty string', () => {
252+
expect(validateApiHost('')).toBe(false);
253+
});
254+
});

packages/core/src/plugins/SegmentDestination.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
SegmentEvent,
1111
UpdateType,
1212
} from '../types';
13-
import { chunk, createPromise, getURL } from '../util';
13+
import { chunk, createPromise, getURL, validateApiHost } from '../util';
1414
import { uploadEvents } from '../api';
1515
import type { SegmentClient } from '../analytics';
1616
import { DestinationMetadataEnrichment } from './DestinationMetadataEnrichment';
@@ -446,8 +446,13 @@ export class SegmentDestination extends DestinationPlugin {
446446
segmentSettings?.apiHost !== undefined &&
447447
segmentSettings?.apiHost !== null
448448
) {
449-
//assign the api host from segment settings (domain/v1)
450-
this.apiHost = `https://${segmentSettings.apiHost}/b`;
449+
if (validateApiHost(segmentSettings.apiHost)) {
450+
this.apiHost = `https://${segmentSettings.apiHost}/b`;
451+
} else {
452+
console.error(
453+
`[Segment] Invalid apiHost "${segmentSettings.apiHost}" received from settings — ignoring, using default endpoint.`
454+
);
455+
}
451456
}
452457

453458
// Read httpConfig: prefer integration-level settings from CDN, fall back to

packages/core/src/plugins/__tests__/SegmentDestination.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,28 @@ describe('SegmentDestination', () => {
375375
});
376376
});
377377

378+
it('ignores a tampered apiHost containing a scheme or query string and falls back to default', async () => {
379+
const events = [{ messageId: 'msg-1' }] as SegmentEvent[];
380+
381+
for (const badHost of [
382+
'https://attacker.com/collect',
383+
'http://attacker.com',
384+
'attacker.com/collect?x=',
385+
'user:pass@attacker.com',
386+
]) {
387+
const { plugin, sendEventsSpy } = createTestWith({
388+
events,
389+
settings: { apiKey: '', apiHost: badHost },
390+
});
391+
jest.spyOn(console, 'error').mockImplementation(jest.fn());
392+
await plugin.flush();
393+
expect(sendEventsSpy).toHaveBeenCalledWith(
394+
expect.objectContaining({ url: defaultApiHost })
395+
);
396+
sendEventsSpy.mockClear();
397+
}
398+
});
399+
378400
it.each([
379401
[false, false], // No proxy, No segment endpoint
380402
[false, true], // No proxy, Yes segment endpoint

packages/core/src/util.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,16 @@ export const createPromise = <T>(
258258
};
259259
};
260260

261+
// Accepts a bare host[/path] value (e.g. "api.segment.io/v1") as supplied by
262+
// the settings CDN. Rejects anything that contains a scheme, credentials,
263+
// query string, or fragment — all of which could redirect uploads to an
264+
// attacker-controlled endpoint when interpolated into `https://${apiHost}/b`.
265+
export function validateApiHost(apiHost: string): boolean {
266+
return /^[a-zA-Z0-9][a-zA-Z0-9._-]*(:\d{2,5})?(\/[a-zA-Z0-9._\-/]*)?$/.test(
267+
apiHost
268+
);
269+
}
270+
261271
export function getURL(host: string, path: string, allowInsecure = false) {
262272
if (!host.startsWith('https://') && !host.startsWith('http://')) {
263273
host = 'https://' + host;

0 commit comments

Comments
 (0)