From 9209bc273bf524f976d084017f0244a238c66e0a Mon Sep 17 00:00:00 2001 From: Michael McShinsky Date: Wed, 5 Aug 2026 11:18:15 -0700 Subject: [PATCH] fix(express): redact signing secrets from errors Remove request-provided signing secrets from Express error responses and 5xx logs while preserving non-sensitive diagnostic context. #9429 --- modules/express/src/clientRoutes.ts | 60 ++++++++++++++++--- .../express/test/unit/clientRoutes/index.ts | 32 ++++++++++ 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/modules/express/src/clientRoutes.ts b/modules/express/src/clientRoutes.ts index 598c0f6ee7..fad2027e21 100755 --- a/modules/express/src/clientRoutes.ts +++ b/modules/express/src/clientRoutes.ts @@ -1689,7 +1689,49 @@ interface RequestHandler extends express.RequestHandler; } -function handleRequestHandlerError(res: express.Response, error: unknown) { +const SENSITIVE_REQUEST_KEYS = new Set([ + 'password', + 'passphrase', + 'walletpassphrase', + 'prv', + 'privatekey', + 'encryptedprv', + 'secret', +]); + +function collectSensitiveRequestValues(value: unknown, values = new Set()): Set { + if (Array.isArray(value)) { + value.forEach((item) => collectSensitiveRequestValues(item, values)); + } else if (value !== null && typeof value === 'object') { + 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); + }); + } + return values; +} + +function redactSensitiveValues(value: unknown, sensitiveValues: Set): 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; +} + +function handleRequestHandlerError(res: express.Response, error: unknown, requestBody?: unknown) { let err; if (error instanceof Error) { err = error; @@ -1699,11 +1741,11 @@ function handleRequestHandlerError(res: express.Response, error: unknown) { err = new BitGoExpressError('(object_error) ' + JSON.stringify(error)); } - const message = err.message || 'local error'; - // use attached result, or make one - let result = err.result || { error: message }; + const sensitiveValues = collectSensitiveRequestValues(requestBody); + const message = (redactSensitiveValues(err.message, sensitiveValues) as string) || 'local error'; + let result = redactSensitiveValues(err.result || { error: message }, sensitiveValues); result = _.extend({}, result, { - message: err.message, + message, name: err.name || 'BitGoExpressError', bitgoJsVersion: version, bitgoExpressVersion: pjson.version, @@ -1711,13 +1753,13 @@ function handleRequestHandlerError(res: express.Response, error: unknown) { }); const status = err.status || 500; if (!(status >= 200 && status < 300)) { - console.log('error %s: %s', status, err.message); + console.log('error %s: %s', status, message); } if (status >= 500 && status <= 599) { if (err.response && err.response.request) { console.log(`failed to make ${err.response.request.method} request to ${err.response.request.url}`); } - console.log(err.stack); + console.log(redactSensitiveValues(err.stack, sensitiveValues)); } res.status(status).send(result); } @@ -1738,7 +1780,7 @@ export function promiseWrapper(promiseRequestHandler: RequestHandler) { res.status(200).send(result); } } catch (e) { - handleRequestHandlerError(res, e); + handleRequestHandlerError(res, e, req.body); } }; } @@ -1755,7 +1797,7 @@ export function typedPromiseWrapper(promiseRequestHandler: TypedRequestHandler) res.status(200).send(result); } } catch (e) { - handleRequestHandlerError(res, e); + handleRequestHandlerError(res, e, req.body); } }; } diff --git a/modules/express/test/unit/clientRoutes/index.ts b/modules/express/test/unit/clientRoutes/index.ts index c035cc3590..5c8abff731 100644 --- a/modules/express/test/unit/clientRoutes/index.ts +++ b/modules/express/test/unit/clientRoutes/index.ts @@ -180,5 +180,37 @@ describe('common methods', () => { res.status.calledWith(500).should.be.true(); res.send.calledWithMatch((result: any) => result.name === 'BitGoExpressError').should.be.true(); }); + + it('should redact signing secrets from error responses and logs', async () => { + const privateKey = 'private-key-value'; + const walletPassphrase = 'wallet-passphrase-value'; + const error = Object.assign(new Error(`failed with ${privateKey} and ${walletPassphrase}`), { + result: { prv: privateKey, walletPassphrase }, + }); + const handler = sandbox.stub().rejects(error); + const req: any = { + body: { + prv: privateKey, + walletPassphrase, + }, + }; + const res: any = { + status: sandbox.stub().returnsThis(), + send: sandbox.stub().returnsThis(), + }; + const next = sandbox.stub(); + const consoleLog = sandbox.stub(console, 'log'); + + await promiseWrapper(handler)(req, res, next); + + const response = res.send.firstCall.args[0]; + response.message.should.equal('failed with [REDACTED] and [REDACTED]'); + response.prv.should.equal('[REDACTED]'); + response.walletPassphrase.should.equal('[REDACTED]'); + JSON.stringify(response).should.not.containEql(privateKey); + JSON.stringify(response).should.not.containEql(walletPassphrase); + consoleLog.args.flat().join(' ').should.not.containEql(privateKey); + consoleLog.args.flat().join(' ').should.not.containEql(walletPassphrase); + }); }); });