Repository navigation
fix(express): handle circular request bodies in error redaction - #10032
MohammedRyaan786 wants to merge 1 commit into
Conversation
445f797 to
4817b69
Compare
|
@claude please review the PR for correctness and provide a verdict |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
@claude review thoroughly and give approval rating |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
|
||
| function redactSensitiveValues(value: unknown, sensitiveValues: Set<string>): unknown { | ||
| function redactSensitiveValues( | ||
| value: unknown, |
There was a problem hiding this comment.
Could you please add a test for a mutated body, to show that secrets are still redacted after the SDK changes the cloned params.
There was a problem hiding this comment.
Added in ddb9fad22: modules/express/test/unit/typedRoutes/sendmany.ts → "should redact secrets when sendMany mutates the cloned params and then fails". The test goes through the real /sendmany route, so it uses the actual createSendParams clone. The mocked wallet.sendMany behaves like the SDK: it attaches a self-referencing wallet to the params (as Object.assign(params, { wallet: this }) does), deletes walletPassphrase from them, then throws an error that contains the passphrase.
The test checks that:
- the response comes back as a 400;
- the error message is
failed to sign with [REDACTED]; - the passphrase appears in neither the response body nor the logs;
- the SDK received a copy (with
walletattached and the passphrase removed), not the request body itself.
I confirmed it fails when createSendParams returns req.body directly: the passphrase leaks unredacted.
Generated by Claude Code
4817b69 to
ddb9fad
Compare
Ticket: COINS-1940 Co-authored-by: Ralph CLI <ralph-cli@bitgo.com> Ralph: 18.12.3-bitgo.3 openrouter/z-ai/glm-5.3-flash@smol openrouter/google/gemini-3.8-flash@commit openrouter/z-ai/glm-5.3@task+plan openrouter/openai/gpt-5.6-luna@advisor+slow openrouter/openai/gpt-5.6-luna-pro@default
ddb9fad to
9621bbd
Compare
This branch has not been deployed
2 participants
Ticket: COINS-1940
This pull request improves the handling of sensitive values in request bodies, specifically addressing issues with circular references that could cause infinite recursion. It also adds a test to ensure that circular request bodies are handled correctly during error redaction.
Improvements to sensitive value collection:
collectSensitiveRequestValuesfunction inclientRoutes.tsto detect and safely handle circular references by tracking visited objects with aWeakSet, preventing infinite recursion when traversing request bodies.Testing enhancements:
clientRoutes/index.tsto verify that the error redaction logic works correctly with circular request bodies, ensuring sensitive values are properly redacted without causing runtime errors.