fix(core): Strip leading BOM before parsing webhook request bodies (#35933)

packages/cli/src/middlewares/body-parser.ts

@@ -23,6 +23,14 @@ const xmlParser = new XmlParser({
 
 const payloadSizeMax = Container.get(GlobalConfig).endpoints.payloadSizeMax;
 
+/**
+ * A leading byte order mark is not valid JSON, but RFC 8259 §8.1 permits
+ * implementations to ignore it rather than treating it as an error. Both the
+ * UTF-8 (EF BB BF) and UTF-16 (FF FE) BOMs decode to U+FEFF, so stripping the
+ * decoded character covers every declared charset.
+ */
+const stripBom = (text: string) => (text.charCodeAt(0) === 0xfeff ? text.slice(1) : text);
+
 const isClientAbortError = (error: unknown): boolean =>
 	error instanceof Error &&
 	'type' in error &&
@@ -87,12 +95,12 @@ export const parseBody = async (req: Request) => {
 		try {
 			if (contentType === 'application/json') {
 				// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
-				req.body = jsonParse(rawBody.toString(encoding));
+				req.body = jsonParse(stripBom(rawBody.toString(encoding)));
 			} else if (contentType?.endsWith('/xml') || contentType?.endsWith('+xml')) {
 				// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
 				req.body = await xmlParser.parseStringPromise(rawBody.toString(encoding));
 			} else if (contentType === 'application/x-www-form-urlencoded') {
-				req.body = parseQueryString(rawBody.toString(encoding), undefined, undefined, {
+				req.body = parseQueryString(stripBom(rawBody.toString(encoding)), undefined, undefined, {
 					maxKeys: 1000,
 				});
 			} else if (contentType === 'text/plain') {

packages/cli/test/integration/middlewares/body-parser.test.ts

@@ -5,7 +5,8 @@ import request from 'supertest';
 import { gzipSync, deflateSync } from 'zlib';
 
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
-import { rawBodyReader, bodyParser } from '@/middlewares/body-parser';
+import { UnprocessableRequestError } from '@/errors/response-errors/unprocessable.error';
+import { rawBodyReader, bodyParser, parseBody } from '@/middlewares/body-parser';
 
 describe('bodyParser', () => {
 	const server = createServer((req, res) => {
@@ -65,4 +66,80 @@ describe('bodyParser', () => {
 
 		await expect(req.readRawBody()).rejects.toThrow(BadRequestError);
 	});
+
+	describe('leading byte order mark', () => {
+		const utf8Bom = Buffer.from([0xef, 0xbb, 0xbf]);
+		const utf16leBom = Buffer.from([0xff, 0xfe]);
+
+		// supertest/superagent JSON-serializes any non-string `.send()` payload
+		// whose content-type matches a registered serializer (application/json,
+		// x-www-form-urlencoded), which would silently mangle these BOM-prefixed
+		// buffers. An identity `.serialize()` override keeps the raw bytes intact
+		// on the wire — necessary for a byte-for-byte BOM test.
+		const identitySerializer = ((data: unknown) => data) as (obj: unknown) => string;
+		const sendRaw = (body: Buffer) =>
+			request(server).post('/').serialize(identitySerializer).send(body);
+
+		it('should strip a UTF-8 BOM before parsing an application/json body', async () => {
+			const body = Buffer.concat([utf8Bom, Buffer.from(JSON.stringify({ hello: 'world' }))]);
+			const response = await sendRaw(body).set('content-type', 'application/json').expect(200);
+			expect(response.text).toEqual('{"hello":"world"}');
+		});
+
+		it('should parse an application/json body without a BOM unchanged', async () => {
+			const body = Buffer.from(JSON.stringify({ hello: 'world' }));
+			const response = await sendRaw(body).set('content-type', 'application/json').expect(200);
+			expect(response.text).toEqual('{"hello":"world"}');
+		});
+
+		it('should strip a UTF-16LE BOM before parsing an application/json body', async () => {
+			const body = Buffer.concat([
+				utf16leBom,
+				Buffer.from(JSON.stringify({ hello: 'world' }), 'utf16le'),
+			]);
+			const response = await sendRaw(body)
+				.set('content-type', 'application/json; charset=utf-16le')
+				.expect(200);
+			expect(response.text).toEqual('{"hello":"world"}');
+		});
+
+		it('should strip a BOM before parsing an x-www-form-urlencoded body', async () => {
+			const body = Buffer.concat([utf8Bom, Buffer.from('a=1')]);
+			const response = await sendRaw(body)
+				.set('content-type', 'application/x-www-form-urlencoded')
+				.expect(200);
+			expect(JSON.parse(response.text)).toEqual({ a: '1' });
+		});
+
+		it('should not crash on a body that is only a BOM', async () => {
+			// The bare http.Server test harness above has no error-handling
+			// middleware to turn a thrown ResponseError into an HTTP response
+			// (that's the real app's job), so this asserts against parseBody
+			// directly rather than hanging the request/response round trip.
+			const req = {
+				contentType: 'application/json',
+				encoding: 'utf8',
+				rawBody: utf8Bom,
+				readRawBody: async () => {},
+			} as unknown as Request;
+
+			await expect(parseBody(req)).rejects.toThrow(UnprocessableRequestError);
+		});
+
+		it('should not strip a BOM character inside a JSON string value', async () => {
+			const valueWithEmbeddedBom = String.fromCharCode(0xfeff) + 'world';
+			const body = Buffer.concat([
+				utf8Bom,
+				Buffer.from(JSON.stringify({ hello: valueWithEmbeddedBom })),
+			]);
+			const response = await sendRaw(body).set('content-type', 'application/json').expect(200);
+			expect(JSON.parse(response.text)).toEqual({ hello: valueWithEmbeddedBom });
+		});
+
+		it('should parse an application/xml body with a leading BOM unchanged', async () => {
+			const body = Buffer.concat([utf8Bom, Buffer.from('<test><a>1</a></test>')]);
+			const response = await sendRaw(body).set('content-type', 'application/xml').expect(200);
+			expect(JSON.parse(response.text)).toEqual({ test: { a: '1' } });
+		});
+	});
 });