fix(express): handle circular request bodies in error redaction

modules/express/src/clientRoutes.ts

@@ -1163,14 +1163,18 @@ async function handleV2AccelerateTransaction(req: ExpressApiRouteRequest<'expres
   return wallet.accelerateTransaction(createSendParams(req));
 }
 
+function cloneRequestBody(req: express.Request) {
+  return req.body !== null && typeof req.body === 'object' && !Array.isArray(req.body) ? { ...req.body } : req.body;
+}
+
 function createSendParams(req: express.Request) {
   if (req.config?.externalSignerUrl !== undefined) {
     return {
       ...req.body,
       customSigningFunction: createCustomSigningFunction(req.config.externalSignerUrl),
     };
   } else {
-    return req.body;
+    return cloneRequestBody(req);
   }
 }
 
@@ -1241,7 +1245,7 @@ function createTSSSendParams(req: express.Request, wallet: Wallet) {
       throw new Error(`MPC Algorithm ${coin.getMPCAlgorithm()} is not supported.`);
     }
   } else {
-    return req.body;
+    return cloneRequestBody(req);
   }
 }
 
@@ -1846,36 +1850,59 @@ interface RequestHandler extends express.RequestHandler<ParamsDictionary, any, R
     | Promise<RequestHandlerResponse>;
 }
 
-function collectSensitiveRequestValues(value: unknown, values = new Set<string>()): Set<string> {
+function isTraversableObject(value: unknown): value is Record<string, unknown> | unknown[] {
+  return value !== null && typeof value === 'object';
+}
+
+function collectSensitiveRequestValues(
+  value: unknown,
+  values = new Set<string>(),
+  seen = new WeakSet<Record<string, unknown> | unknown[]>()
+): Set<string> {
+  if (!isTraversableObject(value) || seen.has(value)) {
+    return values;
+  }
+  seen.add(value);
   if (Array.isArray(value)) {
-    value.forEach((item) => collectSensitiveRequestValues(item, values));
-  } else if (value !== null && typeof value === 'object') {
+    value.forEach((item) => collectSensitiveRequestValues(item, values, seen));
+  } else {
     Object.entries(value).forEach(([key, item]) => {
       if (SENSITIVE_REQUEST_KEYS.has(key.toLowerCase()) && typeof item === 'string' && item.length > 0) {
         values.add(item);
       }
-      collectSensitiveRequestValues(item, values);
+      collectSensitiveRequestValues(item, values, seen);
     });
   }
   return values;
 }
 
-function redactSensitiveValues(value: unknown, sensitiveValues: Set<string>): unknown {
+function redactSensitiveValues(
+  value: unknown,
+  sensitiveValues: Set<string>,
+  ancestors = new WeakSet<Record<string, unknown> | unknown[]>()
+): unknown {
   if (typeof value === 'string') {
     return [...sensitiveValues].reduce((redacted, secret) => redacted.split(secret).join('[REDACTED]'), value);
   }
-  if (Array.isArray(value)) {
-    return value.map((item) => redactSensitiveValues(item, sensitiveValues));
-  }
-  if (value !== null && typeof value === 'object') {
-    return Object.fromEntries(
-      Object.entries(value).map(([key, item]) => [
-        key,
-        SENSITIVE_REQUEST_KEYS.has(key.toLowerCase()) ? '[REDACTED]' : redactSensitiveValues(item, sensitiveValues),
-      ])
-    );
-  }
-  return value;
+  if (!isTraversableObject(value)) {
+    return value;
+  }
+  if (ancestors.has(value)) {
+    return '[Circular]';
+  }
+  ancestors.add(value);
+  const redacted = Array.isArray(value)
+    ? value.map((item) => redactSensitiveValues(item, sensitiveValues, ancestors))
+    : Object.fromEntries(
+        Object.entries(value).map(([key, item]) => [
+          key,
+          SENSITIVE_REQUEST_KEYS.has(key.toLowerCase())
+            ? '[REDACTED]'
+            : redactSensitiveValues(item, sensitiveValues, ancestors),
+        ])
+      );
+  ancestors.delete(value);
+  return redacted;
 }
 
 function handleRequestHandlerError(res: express.Response, error: unknown, requestBody?: unknown) {

modules/express/test/unit/clientRoutes/index.ts

@@ -212,5 +212,29 @@ describe('common methods', () => {
       consoleLog.args.flat().join(' ').should.not.containEql(privateKey);
       consoleLog.args.flat().join(' ').should.not.containEql(walletPassphrase);
     });
+    it('should handle circular request bodies while redacting errors', async () => {
+      const passphrase = 'circular-passphrase';
+      const requestBody: { credentials: { passphrase: string }; self?: unknown } = {
+        credentials: { passphrase },
+      };
+      requestBody.self = requestBody;
+      const errorResult: { secret: string; self?: unknown } = { secret: passphrase };
+      errorResult.self = errorResult;
+      const handler = sandbox
+        .stub()
+        .rejects(Object.assign(new Error(`failed with ${passphrase}`), { result: errorResult }));
+      const req = { body: requestBody } as express.Request;
+      const status = sandbox.stub().returnsThis();
+      const send = sandbox.stub().returnsThis();
+      const res = { status, send } as unknown as express.Response;
+
+      const next: express.NextFunction = () => undefined;
+      await promiseWrapper(handler)(req, res, next);
+
+      status.calledWith(500).should.be.true();
+      send.firstCall.args[0].message.should.equal('failed with [REDACTED]');
+      send.firstCall.args[0].secret.should.equal('[REDACTED]');
+      send.firstCall.args[0].self.should.equal('[Circular]');
+    });
   });
 });

modules/express/test/unit/typedRoutes/sendmany.ts

@@ -365,6 +365,50 @@ describe('SendMany V2 codec tests', function () {
       result.body.should.have.property('error');
     });
 
+    it('should redact secrets when sendMany mutates the cloned params and then fails', async function () {
+      const walletPassphrase = 'test_passphrase_12345';
+      const requestBody = {
+        recipients: [
+          { address: 'mzKTJw3XJNb7VfkFP77mzPJJz4Dkp4M1T6', amount: 1000000 },
+          { address: 'mvQewFHmFjJVr5G7K9TJWNQxB7cLGhJpJV', amount: 500000 },
+        ],
+        walletPassphrase,
+      };
+
+      // Mimic the SDK: attach a self-referencing wallet to the params (as Wallet.prebuildTransaction
+      // does with Object.assign(params, { wallet: this })), drop the passphrase from them, then fail
+      // with an error that leaks the passphrase.
+      let sdkParams: Record<string, unknown> | undefined;
+      const sendMany = sinon.stub().callsFake(async (params: Record<string, unknown>) => {
+        sdkParams = params;
+        const circularWallet: Record<string, unknown> = { id: walletId };
+        circularWallet.self = circularWallet;
+        Object.assign(params, { wallet: circularWallet });
+        delete params.walletPassphrase;
+        throw new Error(`failed to sign with ${walletPassphrase}`);
+      });
+      const mockWallet = { sendMany, _wallet: { multisigType: 'onchain' } };
+      const mockCoin = { wallets: sinon.stub().returns({ get: sinon.stub().resolves(mockWallet) }) };
+      sinon.stub(BitGo.prototype, 'coin').returns(mockCoin as unknown as ReturnType<BitGo['coin']>);
+      const consoleLog = sinon.stub(console, 'log');
+
+      const result = await agent
+        .post(`/api/v2/${coin}/wallet/${walletId}/sendmany`)
+        .set('Authorization', 'Bearer test_access_token_12345')
+        .set('Content-Type', 'application/json')
+        .send(requestBody);
+      consoleLog.restore();
+
+      assert.strictEqual(result.status, 400);
+      assert.strictEqual(result.body.message, 'failed to sign with [REDACTED]');
+      assert.ok(!JSON.stringify(result.body).includes(walletPassphrase));
+      assert.ok(!consoleLog.args.flat().join(' ').includes(walletPassphrase));
+      // The SDK received a copy of the request body, not the body itself
+      assert.ok(sdkParams);
+      assert.ok(sdkParams.wallet);
+      assert.strictEqual(sdkParams.walletPassphrase, undefined);
+    });
+
     it('should support token transfer parameters', async function () {
       const requestBody = {
         recipients: [