feat(logger): enhance sanitize to handle Date, BigInt, and string types and add the test file.Reduce the depth from 50 to 25.#8231
feat(logger): enhance sanitize to handle Date, BigInt, and string types and add the test file.Reduce the depth from 50 to 25.#8231rishikeshdadam136 wants to merge 1 commit intomasterfrom
Conversation
|
@copilot Review it as 10x engineer. |
|
@rishikeshdadam136 I've opened a new pull request, #8232, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@claude review it as 10x engineer |
modules/logger/src/sanitizeLog.ts
Outdated
| } | ||
|
|
||
| if (typeof obj === 'string') { | ||
| return isBearerV2Token(obj) ? '<REMOVED>' : obj; |
There was a problem hiding this comment.
why only bearerV2? shouldn't we remove all sensitive fields?
There was a problem hiding this comment.
Because it has a recognizable format (v2x + hex) that can be detected from the string content alone. For other sensitive values (passwords, private keys, etc.), we can only identify them when they appear under a sensitive key name, which is already handled in the object iteration loop.
if u suggest me any particular format like bearer i will add that also.
Thank you.
There was a problem hiding this comment.
I see
xprv is a pattern that we detect as a prefix as well
There was a problem hiding this comment.
add a function isSensitiveStringValue(s: string) that does the v2x and xprv prefix checks
There was a problem hiding this comment.
Will look into it
Very Thanks
modules/logger/src/sanitizeLog.ts
Outdated
| } | ||
|
|
||
| if (typeof obj === 'string') { | ||
| return isBearerV2Token(obj) ? '<REMOVED>' : obj; |
There was a problem hiding this comment.
add a function isSensitiveStringValue(s: string) that does the v2x and xprv prefix checks
rishikeshdadam136
force-pushed
the
WP-8110
branch
from
b55cda9 to
7425906
Compare
modules/logger/src/sanitizeLog.ts
Outdated
|
|
||
| // Handle Date objects | ||
| if (obj instanceof Date) { | ||
| return obj.toISOString(); |
There was a problem hiding this comment.
| return obj.toISOString(); | |
| return isNaN(obj.getTime()) ? '[Invalid Date]' : obj.toISOString(); |
There was a problem hiding this comment.
Will look into it.
| ]); | ||
|
|
||
| const BEARER_V2_PATTERN = /^v2x[a-f0-9]{32,}$/i; | ||
| const SENSITIVE_PREFIXES = ['v2x', 'xprv']; |
There was a problem hiding this comment.
The old implementation used a case-insensitive regex , so tokens like V2Xaabb... were caught. The new startsWith('v2x') check is case-sensitive, so mixed-case tokens will slip through unredacted?
There was a problem hiding this comment.
Yes will slip through but normally the tokens in the logging statements will be in this format(v2xaabb..) only right,i mean no capital alphabet.
rishikeshdadam136
force-pushed
the
WP-8110
branch
from
8ba8ea6 to
da95bf5
Compare
TICKET-8110
The sanitize() function in @bitgo/logger handles the below ones correctly and added the test files.
-Reduce the depth.