From 2019b7acabe77a355ff0e8c5590c4d2ed4612b8c Mon Sep 17 00:00:00 2001 From: Mohammed Ryaan Date: Thu, 5 Feb 2026 18:09:27 +0530 Subject: [PATCH] feat: capture raw body bytes for v4 hmac calculation TICKET: CAAS-659 --- modules/express/src/clientRoutes.ts | 49 ++- modules/express/src/expressApp.ts | 13 +- .../test/unit/clientRoutes/rawBodyBuffer.ts | 321 ++++++++++++++++++ modules/express/types/express/index.d.ts | 6 + modules/sdk-api/src/api.ts | 72 +++- modules/sdk-api/test/unit/api.ts | 181 +++++++++- 6 files changed, 617 insertions(+), 25 deletions(-) create mode 100644 modules/express/test/unit/clientRoutes/rawBodyBuffer.ts diff --git a/modules/express/src/clientRoutes.ts b/modules/express/src/clientRoutes.ts index ea0b26e20d..af0e066aeb 100755 --- a/modules/express/src/clientRoutes.ts +++ b/modules/express/src/clientRoutes.ts @@ -1192,6 +1192,38 @@ async function handleNetworkV1EnterpriseClientConnections( return handleProxyReq(req, res, next); } +/** + * Helper to send request body, using raw bytes when available. + * + * For v4 HMAC authentication, we need to send the exact bytes that were + * received from the client to ensure the HMAC signature matches. + * The rawBodyBuffer is captured by body-parser's verify callback before + * JSON parsing, preserving exact whitespace, key ordering, etc. + * + * For v2/v3, sending the raw string also works because serializeRequestData + * now properly returns strings as-is for HMAC calculation. + * + * @param request - The superagent request object + * @param req - The Express request containing body and rawBodyBuffer + * @returns The request with body attached + */ +function sendRequestBody(request: ReturnType, req: express.Request) { + if (req.rawBodyBuffer) { + // Preserve original Content-Type header from client + const contentTypeHeader = req.headers['content-type']; + if (contentTypeHeader) { + request.set('Content-Type', Array.isArray(contentTypeHeader) ? contentTypeHeader[0] : contentTypeHeader); + } + // Send raw body as UTF-8 string to preserve exact bytes for HMAC. + // JSON is always UTF-8 (RFC 8259), so this is lossless for JSON bodies. + // serializeRequestData will return this string as-is for HMAC calculation. + return request.send(req.rawBodyBuffer.toString('utf8')); + } + + // Fall back to parsed body for backward compatibility (e.g., non-JSON bodies) + return request.send(req.body); +} + /** * Redirect a request using the bitgo request functions. * @param bitgo @@ -1214,19 +1246,19 @@ export function redirectRequest( request = bitgo.get(url); break; case 'POST': - request = bitgo.post(url).send(req.body); + request = sendRequestBody(bitgo.post(url), req); break; case 'PUT': - request = bitgo.put(url).send(req.body); + request = sendRequestBody(bitgo.put(url), req); break; case 'PATCH': - request = bitgo.patch(url).send(req.body); + request = sendRequestBody(bitgo.patch(url), req); break; case 'OPTIONS': - request = bitgo.options(url).send(req.body); + request = sendRequestBody(bitgo.options(url), req); break; case 'DELETE': - request = bitgo.del(url).send(req.body); + request = sendRequestBody(bitgo.del(url), req); break; } @@ -1267,7 +1299,12 @@ function apiResponse(status: number, result: any, message?: string): ApiResponse return new ApiResponseError(message, status, result); } -const expressJSONParser = bodyParser.json({ limit: '20mb' }); +const expressJSONParser = bodyParser.json({ + limit: '20mb', + verify: (req, res, buf) => { + (req as express.Request).rawBodyBuffer = buf; + }, +}); /** * Perform body parsing here only on routes we want diff --git a/modules/express/src/expressApp.ts b/modules/express/src/expressApp.ts index 1dbc97f30b..ddbe769161 100644 --- a/modules/express/src/expressApp.ts +++ b/modules/express/src/expressApp.ts @@ -302,7 +302,18 @@ export function app(cfg: Config): express.Application { checkPreconditions(cfg); debug('preconditions satisfied'); - app.use(bodyParser.json({ limit: '20mb' })); + app.use( + bodyParser.json({ + limit: '20mb', + verify: (req, res, buf) => { + // Store the raw body buffer on the request object. + // This preserves the exact bytes before JSON parsing, + // which may alter whitespace, key ordering, etc. + // Required for v4 HMAC authentication. + (req as express.Request).rawBodyBuffer = buf; + }, + }) + ); // Be more robust about accepting URLs with double slashes app.use(function replaceUrlSlashes(req, res, next) { diff --git a/modules/express/test/unit/clientRoutes/rawBodyBuffer.ts b/modules/express/test/unit/clientRoutes/rawBodyBuffer.ts new file mode 100644 index 0000000000..6bf26cf9c7 --- /dev/null +++ b/modules/express/test/unit/clientRoutes/rawBodyBuffer.ts @@ -0,0 +1,321 @@ +/** + * Tests for raw body buffer capture functionality + * This ensures exact bytes are preserved for v4 HMAC authentication + * @prettier + */ +import 'should'; +import 'should-http'; +import 'should-sinon'; +import '../../lib/asserts'; +import * as sinon from 'sinon'; +import * as express from 'express'; +import { agent as supertest, Response } from 'supertest'; +import { app as expressApp } from '../../../src/expressApp'; +import { redirectRequest } from '../../../src/clientRoutes'; +import { BitGo } from 'bitgo'; + +describe('Raw Body Buffer Capture', () => { + const sandbox = sinon.createSandbox(); + + afterEach(() => { + sandbox.verifyAndRestore(); + }); + + describe('body-parser verify callback', () => { + let agent: ReturnType; + + beforeEach(() => { + const app = expressApp({ + env: 'test', + disableProxy: true, + } as any); + + // Add a test route that returns the rawBodyBuffer info + app.post('/test/rawbody', (req: express.Request, res: express.Response) => { + res.json({ + hasRawBodyBuffer: !!req.rawBodyBuffer, + rawBodyBufferLength: req.rawBodyBuffer?.length || 0, + rawBodyBufferContent: req.rawBodyBuffer?.toString('utf-8') || null, + parsedBody: req.body, + bodyKeysCount: Object.keys(req.body || {}).length, + }); + }); + + agent = supertest(app); + }); + + it('should capture raw body buffer on POST requests', async () => { + const testBody = { address: 'tb1qtest', amount: 100000 }; + + const res: Response = await agent.post('/test/rawbody').set('Content-Type', 'application/json').send(testBody); + + res.status.should.equal(200); + res.body.hasRawBodyBuffer.should.equal(true); + res.body.rawBodyBufferLength.should.be.greaterThan(0); + res.body.parsedBody.should.deepEqual(testBody); + }); + + it('should preserve exact bytes including whitespace', async () => { + // JSON with extra whitespace that would be lost during parse/re-serialize + const bodyWithWhitespace = '{"address": "tb1qtest", "amount":100000}'; + + const res: Response = await agent + .post('/test/rawbody') + .set('Content-Type', 'application/json') + .send(bodyWithWhitespace); + + res.status.should.equal(200); + // Raw buffer should preserve the exact whitespace + res.body.rawBodyBufferContent.should.equal(bodyWithWhitespace); + // Parsed body should have the correct values + res.body.parsedBody.address.should.equal('tb1qtest'); + res.body.parsedBody.amount.should.equal(100000); + }); + + it('should preserve exact key ordering', async () => { + // JSON with specific key ordering + const bodyWithOrdering = '{"z_last":"last","a_first":"first","m_middle":"middle"}'; + + const res: Response = await agent + .post('/test/rawbody') + .set('Content-Type', 'application/json') + .send(bodyWithOrdering); + + res.status.should.equal(200); + // Raw buffer should preserve exact key ordering + res.body.rawBodyBufferContent.should.equal(bodyWithOrdering); + }); + + it('should handle empty body', async () => { + const res: Response = await agent.post('/test/rawbody').set('Content-Type', 'application/json').send({}); + + res.status.should.equal(200); + res.body.hasRawBodyBuffer.should.equal(true); + res.body.rawBodyBufferLength.should.equal(2); // "{}" + }); + + it('should handle nested JSON objects', async () => { + const nestedBody = { + level1: { + level2: { + level3: { + value: 'deep', + }, + }, + }, + }; + + const res: Response = await agent.post('/test/rawbody').set('Content-Type', 'application/json').send(nestedBody); + + res.status.should.equal(200); + res.body.hasRawBodyBuffer.should.equal(true); + res.body.parsedBody.level1.level2.level3.value.should.equal('deep'); + }); + }); + + describe('redirectRequest with rawBodyBuffer', () => { + // Uses the parent sandbox (no need to create another one) + + it('should use rawBodyBuffer when available for POST requests', async () => { + const bitgo = { + post: sandbox.stub(), + } as unknown as BitGo; + + // Simulate raw body with specific whitespace + const rawBodyWithWhitespace = '{"address": "tb1qtest", "amount":100000}'; + const req = { + body: { address: 'tb1qtest', amount: 100000 }, + rawBodyBuffer: Buffer.from(rawBodyWithWhitespace), + headers: { 'content-type': 'application/json' }, + params: {}, + } as unknown as express.Request; + + let capturedBody: string | undefined; + let capturedContentType: string | undefined; + + const mockRequest = { + set: sandbox.stub().callsFake((header: string, value: string) => { + if (header === 'Content-Type') { + capturedContentType = value; + } + return mockRequest; + }), + send: sandbox.stub().callsFake((body: string) => { + capturedBody = body; + return mockRequest; + }), + result: sandbox.stub().resolves({ success: true }), + res: { statusCode: 200 }, + }; + + (bitgo.post as sinon.SinonStub).returns(mockRequest); + + await redirectRequest(bitgo, 'POST', 'https://example.com/api', req, () => undefined); + + capturedBody!.should.equal(rawBodyWithWhitespace); + capturedContentType!.should.equal('application/json'); + }); + + it('should handle empty body (0-length buffer)', async () => { + const bitgo = { + post: sandbox.stub(), + } as unknown as BitGo; + + const req = { + body: {}, + rawBodyBuffer: Buffer.from(''), + headers: { 'content-type': 'application/json' }, + params: {}, + } as unknown as express.Request; + + let capturedBody: string | undefined; + + const mockRequest = { + set: sandbox.stub().returnsThis(), + send: sandbox.stub().callsFake((body: string) => { + capturedBody = body; + return mockRequest; + }), + result: sandbox.stub().resolves({ success: true }), + res: { statusCode: 200 }, + }; + + (bitgo.post as sinon.SinonStub).returns(mockRequest); + + await redirectRequest(bitgo, 'POST', 'https://example.com/api', req, () => undefined); + + // Should send empty string, not "{}" + capturedBody!.should.equal(''); + }); + + it('should fall back to req.body when rawBodyBuffer is undefined', async () => { + const bitgo = { + post: sandbox.stub(), + } as unknown as BitGo; + + const req = { + body: { address: 'tb1qtest', amount: 100000 }, + // No rawBodyBuffer + headers: { 'content-type': 'application/json' }, + params: {}, + } as unknown as express.Request; + + let capturedBody: unknown; + + const mockRequest = { + set: sandbox.stub().returnsThis(), + send: sandbox.stub().callsFake((body: unknown) => { + capturedBody = body; + return mockRequest; + }), + result: sandbox.stub().resolves({ success: true }), + res: { statusCode: 200 }, + }; + + (bitgo.post as sinon.SinonStub).returns(mockRequest); + + await redirectRequest(bitgo, 'POST', 'https://example.com/api', req, () => undefined); + + // Should fall back to parsed body object + capturedBody!.should.deepEqual({ address: 'tb1qtest', amount: 100000 }); + }); + + it('should preserve original Content-Type header from client', async () => { + const bitgo = { + post: sandbox.stub(), + } as unknown as BitGo; + + const req = { + body: { data: 'test' }, + rawBodyBuffer: Buffer.from('{"data":"test"}'), + headers: { 'content-type': 'application/json; charset=utf-8' }, + params: {}, + } as unknown as express.Request; + + let capturedContentType: string | undefined; + + const mockRequest = { + set: sandbox.stub().callsFake((header: string, value: string) => { + if (header === 'Content-Type') { + capturedContentType = value; + } + return mockRequest; + }), + send: sandbox.stub().returnsThis(), + result: sandbox.stub().resolves({ success: true }), + res: { statusCode: 200 }, + }; + + (bitgo.post as sinon.SinonStub).returns(mockRequest); + + await redirectRequest(bitgo, 'POST', 'https://example.com/api', req, () => undefined); + + // Should preserve original Content-Type including charset + capturedContentType!.should.equal('application/json; charset=utf-8'); + }); + + it('should use rawBodyBuffer for PUT requests', async () => { + const bitgo = { + put: sandbox.stub(), + } as unknown as BitGo; + + const rawBody = '{"update":"value"}'; + const req = { + body: { update: 'value' }, + rawBodyBuffer: Buffer.from(rawBody), + headers: { 'content-type': 'application/json' }, + params: {}, + } as unknown as express.Request; + + let capturedBody: string | undefined; + + const mockRequest = { + set: sandbox.stub().returnsThis(), + send: sandbox.stub().callsFake((body: string) => { + capturedBody = body; + return mockRequest; + }), + result: sandbox.stub().resolves({ success: true }), + res: { statusCode: 200 }, + }; + + (bitgo.put as sinon.SinonStub).returns(mockRequest); + + await redirectRequest(bitgo, 'PUT', 'https://example.com/api', req, () => undefined); + + capturedBody!.should.equal(rawBody); + }); + + it('should use rawBodyBuffer for DELETE requests', async () => { + const bitgo = { + del: sandbox.stub(), + } as unknown as BitGo; + + const rawBody = '{"id":"123"}'; + const req = { + body: { id: '123' }, + rawBodyBuffer: Buffer.from(rawBody), + headers: { 'content-type': 'application/json' }, + params: {}, + } as unknown as express.Request; + + let capturedBody: string | undefined; + + const mockRequest = { + set: sandbox.stub().returnsThis(), + send: sandbox.stub().callsFake((body: string) => { + capturedBody = body; + return mockRequest; + }), + result: sandbox.stub().resolves({ success: true }), + res: { statusCode: 200 }, + }; + + (bitgo.del as sinon.SinonStub).returns(mockRequest); + + await redirectRequest(bitgo, 'DELETE', 'https://example.com/api', req, () => undefined); + + capturedBody!.should.equal(rawBody); + }); + }); +}); diff --git a/modules/express/types/express/index.d.ts b/modules/express/types/express/index.d.ts index 54d63df946..f754222bc9 100644 --- a/modules/express/types/express/index.d.ts +++ b/modules/express/types/express/index.d.ts @@ -6,5 +6,11 @@ declare module 'express-serve-static-core' { isProxy: boolean; bitgo: BitGo; config: Config; + /** + * Raw body buffer captured before JSON parsing. + * Used for v4 HMAC authentication to ensure the exact bytes + * sent by the client are used for signature calculation. + */ + rawBodyBuffer?: Buffer; } } diff --git a/modules/sdk-api/src/api.ts b/modules/sdk-api/src/api.ts index 20bcccf04b..0693f1389a 100644 --- a/modules/sdk-api/src/api.ts +++ b/modules/sdk-api/src/api.ts @@ -121,28 +121,66 @@ function createResponseErrorString(res: superagent.Response): string { } /** - * Serialize request data based on the request content type - * Note: Not sure this is still needed or even useful. Consider removing. + * Serialize request data based on the request content type. + * If data is already a string, returns it as-is to preserve exact bytes for HMAC. + * If data is a Buffer, converts to UTF-8 string. + * If data is an object, serializes it based on Content-Type. * @param req */ export function serializeRequestData(req: superagent.Request): string | undefined { - let data: string | Record = (req as any)._data; - if (typeof data !== 'string') { - let contentType = req.get('Content-Type'); - // Parse out just the content type from the header (ignore the charset) - if (contentType) { - contentType = contentType.split(';')[0]; - } - let serialize = superagent.serialize[contentType]; - if (!serialize && /[\/+]json\b/.test(contentType)) { - serialize = superagent.serialize['application/json']; - } - if (serialize) { - data = serialize(data); - (req as any)._data = data; - return data; + const data: string | Buffer | Record | undefined = (req as any)._data; + + if (typeof data === 'string') { + return data; + } + + if (Buffer.isBuffer(data)) { + // Convert Buffer to UTF-8 string and mutate _data to ensure consistency. + // This is critical: if _data stays as a Buffer, superagent will serialize it + // as JSON ({"type":"Buffer","data":[...]}), which won't match the HMAC. + // By mutating to string, superagent sends the exact bytes we hash. + // This is safe for retries: calling again with a string will hit the early return above. + const stringData = data.toString('utf8'); + (req as any)._data = stringData; + return stringData; + } + + if (data === undefined || data === null) { + return undefined; + } + + // Serialize object data based on Content-Type + let contentType = req.get('Content-Type'); + // Parse out just the content type from the header (ignore the charset) + if (contentType) { + contentType = contentType.split(';')[0].trim(); + } + let serialize: ((body: unknown) => string) | undefined; + const serializers = superagent.serialize as Record; + if (contentType) { + if (Object.prototype.hasOwnProperty.call(serializers, contentType)) { + const candidate = serializers[contentType]; + if (typeof candidate === 'function') { + serialize = candidate as (body: unknown) => string; + } + } else if (/[\/+]json\b/.test(contentType)) { + // Fall back to the built-in JSON serializer for JSON-like content types + if (Object.prototype.hasOwnProperty.call(serializers, 'application/json')) { + const jsonSerializer = serializers['application/json']; + if (typeof jsonSerializer === 'function') { + serialize = jsonSerializer as (body: unknown) => string; + } + } } } + + if (serialize) { + const serialized = serialize(data); + (req as any)._data = serialized; + return serialized; + } + + return undefined; } /** diff --git a/modules/sdk-api/test/unit/api.ts b/modules/sdk-api/test/unit/api.ts index 6a804788b7..98eb2f6cce 100644 --- a/modules/sdk-api/test/unit/api.ts +++ b/modules/sdk-api/test/unit/api.ts @@ -1,7 +1,186 @@ import * as assert from 'assert'; -import { handleResponseError, handleResponseResult } from '../../src'; +import { handleResponseError, handleResponseResult, serializeRequestData } from '../../src'; describe('bitgo:api unit tests', function () { + describe('serializeRequestData', function () { + it('should return string data as-is for v4 raw body support', function () { + const stringBody = '{"address": "tb1qtest", "amount":100000}'; + const mockReq: any = { + _data: stringBody, + get: () => 'application/json', + }; + + const result = serializeRequestData(mockReq); + assert.strictEqual(result, stringBody); + // Verify _data was not modified + assert.strictEqual(mockReq._data, stringBody); + }); + + it('should return empty string as-is', function () { + const mockReq: any = { + _data: '', + get: () => 'application/json', + }; + + const result = serializeRequestData(mockReq); + assert.strictEqual(result, ''); + }); + + it('should convert Buffer to UTF-8 string and mutate _data', function () { + const jsonString = '{"address":"tb1qtest","amount":100000}'; + const bufferBody = Buffer.from(jsonString, 'utf8'); + const mockReq: any = { + _data: bufferBody, + get: () => 'application/json', + }; + + const result = serializeRequestData(mockReq); + assert.strictEqual(typeof result, 'string'); + assert.strictEqual(result, jsonString); + // Verify _data WAS mutated to string - critical for superagent to send correct bytes + assert.strictEqual(typeof mockReq._data, 'string'); + assert.strictEqual(mockReq._data, jsonString); + }); + + it('should convert empty Buffer to empty string and mutate _data', function () { + const mockReq: any = { + _data: Buffer.from(''), + get: () => 'application/json', + }; + + const result = serializeRequestData(mockReq); + assert.strictEqual(typeof result, 'string'); + assert.strictEqual(result, ''); + // Verify _data WAS mutated to string + assert.strictEqual(typeof mockReq._data, 'string'); + assert.strictEqual(mockReq._data, ''); + }); + + it('should produce identical results on retry for HMAC consistency', function () { + const jsonString = '{"address":"tb1qtest","amount":100000}'; + const bufferBody = Buffer.from(jsonString, 'utf8'); + const mockReq: any = { + _data: bufferBody, + get: () => 'application/json', + }; + + // First call (original request) - converts Buffer to string, mutates _data + const result1 = serializeRequestData(mockReq); + assert.strictEqual(result1, jsonString); + assert.strictEqual(typeof mockReq._data, 'string'); + + // Second call (simulating retry) - _data is now a string, returns as-is + const result2 = serializeRequestData(mockReq); + + // Both calls should return identical strings for consistent HMAC + assert.strictEqual(result1, result2); + assert.strictEqual(result2, jsonString); + // _data should now be a string (mutated on first call) + assert.strictEqual(typeof mockReq._data, 'string'); + assert.strictEqual(mockReq._data, jsonString); + }); + + it('should mutate Buffer to prevent superagent JSON serialization', function () { + // This test verifies the critical behavior: if we don't mutate _data from Buffer to string, + // superagent will serialize the Buffer as {"type":"Buffer","data":[...]}, which won't match + // the HMAC calculated on the UTF-8 string. By mutating _data, we ensure superagent sends + // the exact bytes we used for HMAC calculation. + const jsonString = '{"address":"tb1qtest","amount":100000}'; + const bufferBody = Buffer.from(jsonString, 'utf8'); + const mockReq: any = { + _data: bufferBody, + get: () => 'application/json', + }; + + serializeRequestData(mockReq); + + // After serialization, _data MUST be a string, not a Buffer + assert.strictEqual(typeof mockReq._data, 'string'); + assert.strictEqual(mockReq._data, jsonString); + + // This ensures when superagent sends the request, it sends the UTF-8 string, + // not the JSON-serialized Buffer representation. + }); + + it('should serialize object data to JSON', function () { + const objectBody = { address: 'tb1qtest', amount: 100000 }; + const mockReq: any = { + _data: objectBody, + get: () => 'application/json', + }; + + const result = serializeRequestData(mockReq); + assert.strictEqual(result, JSON.stringify(objectBody)); + // Verify _data was updated to serialized form + assert.strictEqual(mockReq._data, JSON.stringify(objectBody)); + }); + + it('should handle nested objects', function () { + const nestedBody = { level1: { level2: { value: 'deep' } } }; + const mockReq: any = { + _data: nestedBody, + get: () => 'application/json', + }; + + const result = serializeRequestData(mockReq); + assert.strictEqual(result, JSON.stringify(nestedBody)); + }); + + it('should return undefined for undefined data', function () { + const mockReq: any = { + _data: undefined, + get: () => 'application/json', + }; + + const result = serializeRequestData(mockReq); + assert.strictEqual(result, undefined); + }); + + it('should return undefined for null data', function () { + const mockReq: any = { + _data: null, + get: () => 'application/json', + }; + + const result = serializeRequestData(mockReq); + assert.strictEqual(result, undefined); + }); + + it('should handle Content-Type with charset', function () { + const objectBody = { test: 'value' }; + const mockReq: any = { + _data: objectBody, + get: () => 'application/json; charset=utf-8', + }; + + const result = serializeRequestData(mockReq); + assert.strictEqual(result, JSON.stringify(objectBody)); + }); + + it('should handle application/vnd.api+json content type', function () { + const objectBody = { data: 'test' }; + const mockReq: any = { + _data: objectBody, + get: () => 'application/vnd.api+json', + }; + + const result = serializeRequestData(mockReq); + assert.strictEqual(result, JSON.stringify(objectBody)); + }); + + it('should return undefined for unsupported content type without serializer', function () { + const objectBody = { test: 'value' }; + const mockReq: any = { + _data: objectBody, + get: () => 'text/plain', + }; + + const result = serializeRequestData(mockReq); + // text/plain doesn't have a JSON serializer, so returns undefined + assert.strictEqual(result, undefined); + }); + }); + describe('handleResponseResult', function () { it('should return text for text-based responses', function () { const csvText = `transactionId,date,amount,currency,status