Redact sensitive information #1681
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a redaction subsystem to the log cleaner: new helpers detect sensitive keys and patterns, redact nested and circular objects, and redact sensitive substrings (including stringified JSON and error stacks); integrates redaction with existing truncation and stringify flows without changing exported signatures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant Cleaner as cleanLogEvent()
participant Fields as redactSensitiveFields()
participant StringRedact as redactSensitiveDataInString()
participant Output as Cleaned Log
Caller->>Cleaner: submit raw logEvent
Cleaner->>Cleaner: inspect message, event data, error, params, context
alt key matches sensitive pattern
Cleaner->>Output: replace value with "REDACTED"
else value is object
Cleaner->>Fields: recursively redact sensitive keys (handles cycles)
Fields-->>Cleaner: return redacted object
Cleaner->>Cleaner: stringify & truncate redacted object
else value is string
Cleaner->>StringRedact: redact sensitive substrings (incl. JSON/stacks)
StringRedact-->>Cleaner: return masked string
Cleaner->>Cleaner: truncate masked string if needed
end
Cleaner->>Output: emit cleaned, redacted logEvent
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
|
||
| const REDACTED = '[REDACTED]'; | ||
|
|
||
| const SENSITIVE_FIELDS = [ |
There was a problem hiding this comment.
idk i asked the ai to come up with a list, maybe its a bit much
There was a problem hiding this comment.
you should redact any key that includes the word "password" so you don't need all the various variations
There was a problem hiding this comment.
Pull request overview
This PR implements sensitive data redaction in logging to prevent sensitive information like passwords, tokens, and authorization headers from being written to logs.
- Adds redaction logic for sensitive fields in log events, error contexts, and stringified JSON
- Introduces a list of sensitive field names that should be automatically redacted
- Applies redaction throughout the logging pipeline including messages, error stacks, and nested objects
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/server/shared/src/lib/logger/log-cleaner.ts | Implements core redaction logic with sensitive field detection, object traversal, and string pattern matching |
| packages/server/shared/test/log-cleaner.test.ts | Adds comprehensive test coverage for password, authorization, and nested object redaction scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile OverviewGreptile SummaryAdded comprehensive sensitive data redaction to the log cleaner, preventing credentials and secrets from appearing in logs.
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant App as Application Code
participant Logger as Pino Logger
participant Cleaner as cleanLogEvent
participant Redact as Redaction Functions
participant Output as Log Output
App->>Logger: logger.info(message, event)
Logger->>Cleaner: cleanLogEvent({message, event})
alt message exists
Cleaner->>Redact: redactSensitiveDataInString(message)
Redact-->>Cleaner: redacted message
end
loop for each event field
alt field is sensitive (password, token, etc)
Cleaner->>Cleaner: Replace with [REDACTED]
else field is Error
Cleaner->>Redact: redactSensitiveDataInString(stack/message)
Redact-->>Cleaner: redacted error data
else field is object
Cleaner->>Redact: redactSensitiveFields(object)
Redact->>Redact: Recursively traverse object
Redact-->>Cleaner: redacted object (stringified)
else field is string
Cleaner->>Redact: redactSensitiveDataInString(value)
Redact-->>Cleaner: redacted string
end
end
Cleaner-->>Logger: cleaned logEvent
Logger->>Output: Write to console/Logz.io
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/server/shared/src/lib/logger/log-cleaner.ts (2)
64-69: UsereplaceAlland address regex construction.Per static analysis hints, prefer
String#replaceAll()overString#replace()with a global regex. Since the regex pattern is constructed from a known constant list (not user input), the ReDoS warning is a false positive here, but usingreplaceAllimproves clarity.SENSITIVE_FIELDS.forEach((field) => { - result = result.replace( - new RegExp(`"${field}"\\s*:\\s*"[^"]*"`, 'gi'), + result = result.replaceAll( + new RegExp(String.raw`"${field}"\s*:\s*"[^"]*"`, 'gi'), `"${field}":"${REDACTED}"`, ); });
105-106: Use optional chaining for cleaner conditional.Per static analysis, prefer optional chaining.
- } else if (key === 'res' && value && value.raw) { + } else if (key === 'res' && value?.raw) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/server/shared/src/lib/logger/log-cleaner.ts(4 hunks)packages/server/shared/test/log-cleaner.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Indentation: 2 spaces for TypeScript/JavaScript
Braces required for all control blocks, even single-line
One space between keywords and parentheses:if (condition) {
Use camelCase for variables and functions
Use PascalCase for classes and types
Use UPPER_SNAKE_CASE for constants
Use lowercase with hyphens for file names (e.g.,user-profile.ts)
Preferconstoverletorvarin TypeScript/JavaScript
Prefer arrow functions for callbacks and functional components in TypeScript/JavaScript
Files:
packages/server/shared/test/log-cleaner.test.tspackages/server/shared/src/lib/logger/log-cleaner.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript
Use explicit return types for exported functions in TypeScript
Files:
packages/server/shared/test/log-cleaner.test.tspackages/server/shared/src/lib/logger/log-cleaner.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{tsx,ts}: Frontend tech stack must strictly use: React 18, Zustand, react-query v5, shadcn, and Axios withqspackage for query strings
Follow best practices for React hooks
Prefer small, composable components and extract helper functions where possible
Usecnutility to group tailwind classnames in React components
Files:
packages/server/shared/test/log-cleaner.test.tspackages/server/shared/src/lib/logger/log-cleaner.ts
🧬 Code graph analysis (1)
packages/server/shared/src/lib/logger/log-cleaner.ts (3)
packages/server/shared/src/lib/logger/index.ts (1)
truncate(112-112)tools/scripts/publish.mjs (1)
name(25-25)packages/shared/src/lib/common/application-error.ts (1)
ApplicationError(10-14)
🪛 ast-grep (0.40.0)
packages/server/shared/src/lib/logger/log-cleaner.ts
[warning] 65-65: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp("${field}"\\s*:\\s*"[^"]*", 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 GitHub Actions: CI
packages/server/shared/src/lib/logger/log-cleaner.ts
[error] 31-31: Maximum call stack size exceeded in redactSensitiveFields (likely due to circular references) during tests.
🪛 GitHub Check: SonarCloud Code Analysis
packages/server/shared/src/lib/logger/log-cleaner.ts
[warning] 65-65: Prefer String#replaceAll() over String#replace().
[warning] 66-66: String.raw should be used to avoid escaping \.
[warning] 105-105: Prefer using an optional chain expression instead, as it's more concise and easier to read.
🔇 Additional comments (4)
packages/server/shared/src/lib/logger/log-cleaner.ts (3)
7-25: LGTM on sensitive fields list and constant.The
REDACTEDmarker andSENSITIVE_FIELDSlist provide good coverage for common authentication and session-related fields.
82-122: Redaction integration incleanLogEventlooks correct.The flow properly handles sensitive field detection at the top-level, applies recursive redaction for objects before stringify, and redacts sensitive data in strings after truncation. The order of operations (truncate → redact for strings, redact → stringify → truncate for objects) is appropriate.
141-167: Redaction inextractErrorFieldsis properly applied.Error stacks, messages, params, and context are all passed through the appropriate redaction functions. This ensures sensitive data doesn't leak through error objects.
packages/server/shared/test/log-cleaner.test.ts (1)
140-199: Good test coverage for basic redaction scenarios.The tests appropriately verify redaction of sensitive fields in various contexts: direct fields, nested objects, and stringified JSON.
Consider adding tests for:
- Circular references - to verify the fix prevents stack overflow:
it('should handle circular references in objects with sensitive fields', () => { const circularObj: any = { password: 'secret', data: {} }; circularObj.data.self = circularObj; const logEvent = { event: { request: circularObj } }; expect(() => cleanLogEvent(logEvent)).not.toThrow(); expect(result.event.request).toContain('[REDACTED]'); });
- Case-insensitive field matching - e.g.,
PASSWORD,Authorization:it('should redact fields case-insensitively', () => { const logEvent = { event: { PASSWORD: 'secret', Authorization: 'Bearer token', }, }; const result = cleanLogEvent(logEvent); expect(result.event.PASSWORD).toBe('[REDACTED]'); expect(result.event.Authorization).toBe('[REDACTED]'); });
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/server/shared/src/lib/logger/log-cleaner.ts (2)
27-29: Fix case-sensitivity bug that breaks redaction.This is a critical bug that has been flagged multiple times:
key.toLowerCase()converts the key to lowercase, butSENSITIVE_FIELDScontains camelCase entries likenewPassword,accessToken, etc. Theincludes()check will never match, so sensitive fields won't be redacted.Apply this diff to fix:
const isSensitiveField = (key: string): boolean => { - return SENSITIVE_FIELDS.includes(key.toLowerCase()); + return SENSITIVE_FIELDS.some( + (field) => field.toLowerCase() === key.toLowerCase(), + ); };
70-77: Regex pattern only redacts double-quoted JSON values.The current regex
"${field}"\\s*:\\s*"[^"]*"only matches JSON-like patterns with double quotes. It will miss:
- URL parameters:
password=secret123&token=abc- Single-quoted values
- Other non-JSON formats
This creates a security gap where sensitive data in these formats won't be redacted from logs.
Consider either:
- Adding additional patterns for URL parameters and other common formats
- Documenting this limitation clearly
- If comprehensive string redaction isn't feasible, focus on object-level redaction (which is already working)
Additionally, apply these code style improvements:
let result = value; SENSITIVE_FIELDS.forEach((field) => { - result = result.replace( - new RegExp(`"${field}"\\s*:\\s*"[^"]*"`, 'gi'), + result = result.replaceAll( + new RegExp(String.raw`"${field}"\s*:\s*"[^"]*"`, 'gi'), `"${field}":"${REDACTED}"`, ); });Based on static analysis hints:
replaceAllis more explicit for global replacements, andString.rawavoids manual escaping.
🧹 Nitpick comments (2)
packages/server/shared/src/lib/logger/log-cleaner.ts (2)
88-88: Add explicit return type to exported function.Per coding guidelines, exported functions should have explicit return types.
Apply this diff:
-export const cleanLogEvent = (logEvent: any) => { +export const cleanLogEvent = (logEvent: any): any => {As per coding guidelines: "Use explicit return types for exported functions in TypeScript"
111-111: Consider using optional chain for cleaner null checking.Apply this diff:
- } else if (key === 'res' && value && value.raw) { + } else if (key === 'res' && value?.raw) {Based on static analysis hints: Optional chain is more concise and idiomatic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/server/shared/src/lib/logger/log-cleaner.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Indentation: 2 spaces for TypeScript/JavaScript
Braces required for all control blocks, even single-line
One space between keywords and parentheses:if (condition) {
Use camelCase for variables and functions
Use PascalCase for classes and types
Use UPPER_SNAKE_CASE for constants
Use lowercase with hyphens for file names (e.g.,user-profile.ts)
Preferconstoverletorvarin TypeScript/JavaScript
Prefer arrow functions for callbacks and functional components in TypeScript/JavaScript
Files:
packages/server/shared/src/lib/logger/log-cleaner.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript
Use explicit return types for exported functions in TypeScript
Files:
packages/server/shared/src/lib/logger/log-cleaner.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{tsx,ts}: Frontend tech stack must strictly use: React 18, Zustand, react-query v5, shadcn, and Axios withqspackage for query strings
Follow best practices for React hooks
Prefer small, composable components and extract helper functions where possible
Usecnutility to group tailwind classnames in React components
Files:
packages/server/shared/src/lib/logger/log-cleaner.ts
🧬 Code graph analysis (1)
packages/server/shared/src/lib/logger/log-cleaner.ts (1)
packages/shared/src/lib/common/application-error.ts (1)
ApplicationError(10-14)
🪛 ast-grep (0.40.0)
packages/server/shared/src/lib/logger/log-cleaner.ts
[warning] 71-71: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp("${field}"\\s*:\\s*"[^"]*", 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 GitHub Check: SonarCloud Code Analysis
packages/server/shared/src/lib/logger/log-cleaner.ts
[warning] 72-72: String.raw should be used to avoid escaping \.
[warning] 71-71: Prefer String#replaceAll() over String#replace().
[warning] 111-111: Prefer using an optional chain expression instead, as it's more concise and easier to read.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test Engine and Libraries
- GitHub Check: Test Blocks Shard 3
- GitHub Check: Test Blocks Shard 2
- GitHub Check: Test Blocks Shard 1
- GitHub Check: Test Server API
- GitHub Check: Build Project
🔇 Additional comments (5)
packages/server/shared/src/lib/logger/log-cleaner.ts (5)
7-25: LGTM: Comprehensive list of sensitive fields.The
SENSITIVE_FIELDSlist covers common authentication and authorization fields appropriately. While the author noted it might be "a bit much," this comprehensive approach is preferable for security.
31-61: Well done: Circular reference handling implemented correctly.The addition of
WeakSettracking and the'[Circular]'placeholder properly prevents stack overflow on circular references. The logic correctly checks for visited objects before recursing and passes thevisitedset through all recursive calls.
90-90: LGTM: Message redaction applied correctly.The chaining of
truncatethenredactSensitiveDataInStringcorrectly processes the log message (subject to the limitations of the regex pattern flagged separately).
117-121: LGTM: Object and string redaction integrated correctly.The logic correctly applies:
- For objects:
redactSensitiveFieldsbefore stringification to handle nested sensitive keys- For strings:
truncatethenredactSensitiveDataInStringto handle embedded sensitive dataThe integration of redaction with existing truncation logic is sound.
155-172: LGTM: Error data properly redacted across all fields.The function correctly applies redaction to
stack,message,params, andcontext.Regarding Line 170: The removed null check for
contextis safe because destructuring with the rest operator (...context) always produces an object, soObject.keys(context)won't throw. The previous defensive check was redundant.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/server/shared/src/lib/logger/log-cleaner.ts (3)
27-29: Consider usingString.rawfor clearer regex escaping.Per static analysis, using
String.rawimproves readability by avoiding double-escaping.const SENSITIVE_FIELD_PATTERNS = SENSITIVE_FIELDS.map( - (field) => new RegExp(`"${field}"\\s*:\\s*"[^"]*"`, 'gi'), + (field) => new RegExp(String.raw`"${field}"\s*:\s*"[^"]*"`, 'gi'), );
117-117: Use optional chaining for cleaner code.Per static analysis, optional chaining is more concise.
- } else if (key === 'res' && value && value.raw) { + } else if (key === 'res' && value?.raw) {
181-183: Consider wrappingJSON.stringifyfor resilience.
JSON.stringifycan throw onBigIntvalues or if atoJSONmethod throws. While unlikely in typical log data, a try/catch could prevent logging failures from crashing the application.function stringify(value: any) { - return truncate(JSON.stringify(value)); + try { + return truncate(JSON.stringify(value)); + } catch { + return '[Unserializable]'; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/server/shared/src/lib/logger/log-cleaner.ts(4 hunks)packages/server/shared/test/log-cleaner.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/shared/test/log-cleaner.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Indentation: 2 spaces for TypeScript/JavaScript
Braces required for all control blocks, even single-line
One space between keywords and parentheses:if (condition) {
Use camelCase for variables and functions
Use PascalCase for classes and types
Use UPPER_SNAKE_CASE for constants
Use lowercase with hyphens for file names (e.g.,user-profile.ts)
Preferconstoverletorvarin TypeScript/JavaScript
Prefer arrow functions for callbacks and functional components in TypeScript/JavaScript
Files:
packages/server/shared/src/lib/logger/log-cleaner.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript
Use explicit return types for exported functions in TypeScript
Files:
packages/server/shared/src/lib/logger/log-cleaner.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{tsx,ts}: Frontend tech stack must strictly use: React 18, Zustand, react-query v5, shadcn, and Axios withqspackage for query strings
Follow best practices for React hooks
Prefer small, composable components and extract helper functions where possible
Usecnutility to group tailwind classnames in React components
Files:
packages/server/shared/src/lib/logger/log-cleaner.ts
🪛 ast-grep (0.40.0)
packages/server/shared/src/lib/logger/log-cleaner.ts
[warning] 27-27: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp("${field}"\\s*:\\s*"[^"]*", 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 GitHub Check: SonarCloud Code Analysis
packages/server/shared/src/lib/logger/log-cleaner.ts
[warning] 28-28: String.raw should be used to avoid escaping \.
[warning] 117-117: Prefer using an optional chain expression instead, as it's more concise and easier to read.
🔇 Additional comments (3)
packages/server/shared/src/lib/logger/log-cleaner.ts (3)
31-35: Case-insensitivity fix looks correct.The implementation now properly handles case-insensitive matching. Minor optimization opportunity: pre-compute a lowercase
Setat module load for O(1) lookups, though with only 16 fields this is negligible.
37-67: Circular reference handling is correctly implemented.The
WeakSetapproach properly prevents stack overflow. One consideration: string values within objects aren't passed throughredactSensitiveDataInString, so stringified JSON embedded in object fields won't have in-string sensitive patterns redacted. This may be acceptable if such strings are rare in your log data.
153-179: Error field extraction with redaction looks correct.The destructuring
...contextguaranteescontextis always an object (possibly empty{}), so the null check isn't needed at line 176. The redaction is consistently applied to stack, message, params, and context.
| ); | ||
| }; | ||
|
|
||
| const redactSensitiveFields = (obj: any, visited = new WeakSet()): any => { |
There was a problem hiding this comment.
never heard of weakset, apparently its more memory efficient? maybe ai comments arent so bad after all
There was a problem hiding this comment.
no idea but can you make sure this function can never throw an error? we simply can't allow the logger to fail. If there's an error then the field should just show the error message.
|
|
||
| const REDACTED = '[REDACTED]'; | ||
|
|
||
| const SENSITIVE_FIELDS = [ |
There was a problem hiding this comment.
you should redact any key that includes the word "password" so you don't need all the various variations
| } else if (typeof value === 'string') { | ||
| eventData[key] = redactSensitiveDataInString(truncate(value)); | ||
| } else { | ||
| eventData[key] = truncate(value); | ||
| } |
There was a problem hiding this comment.
string is pretty much the only type that is left anyway
There was a problem hiding this comment.
hey you never know
| ); | ||
| }; | ||
|
|
||
| const redactSensitiveFields = (obj: any, visited = new WeakSet()): any => { |
There was a problem hiding this comment.
no idea but can you make sure this function can never throw an error? we simply can't allow the logger to fail. If there's an error then the field should just show the error message.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/server/shared/src/lib/logger/log-cleaner.ts (1)
67-82: Consider combining patterns into a single regex for efficiency.The current implementation iterates and creates intermediate strings for each pattern. While functionally correct, a single combined regex would be more efficient for strings with many patterns to check.
const redactSensitiveDataInString = ( value: string | undefined, ): string | undefined => { if (!value) { return value; } - let result = value; - SENSITIVE_FIELD_PATTERNS.forEach((pattern) => { - result = result.replace(pattern, (match) => { - const colonIndex = match.indexOf(':'); - const keyPart = match.substring(0, colonIndex); - return `${keyPart}:"${REDACTED}"`; - }); - }); - return result; + const combinedPattern = new RegExp( + `"[^"]*(${SENSITIVE_PATTERNS.join('|')})[^"]*"\\s*:\\s*"[^"]*"`, + 'gi', + ); + return value.replace(combinedPattern, (match) => { + const colonIndex = match.indexOf(':'); + const keyPart = match.substring(0, colonIndex); + return `${keyPart}:"${REDACTED}"`; + }); };Alternatively, move the combined regex to module scope to avoid recompilation on each call.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/server/shared/src/lib/logger/log-cleaner.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Indentation: 2 spaces for TypeScript/JavaScript
Braces required for all control blocks, even single-line
One space between keywords and parentheses:if (condition) {
Use camelCase for variables and functions
Use PascalCase for classes and types
Use UPPER_SNAKE_CASE for constants
Use lowercase with hyphens for file names (e.g.,user-profile.ts)
Preferconstoverletorvarin TypeScript/JavaScript
Prefer arrow functions for callbacks and functional components in TypeScript/JavaScript
Files:
packages/server/shared/src/lib/logger/log-cleaner.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript
Use explicit return types for exported functions in TypeScript
Files:
packages/server/shared/src/lib/logger/log-cleaner.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{tsx,ts}: Frontend tech stack must strictly use: React 18, Zustand, react-query v5, shadcn, and Axios withqspackage for query strings
Follow best practices for React hooks
Prefer small, composable components and extract helper functions where possible
Usecnutility to group tailwind classnames in React components
Files:
packages/server/shared/src/lib/logger/log-cleaner.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test Server API
- GitHub Check: Test Engine and Libraries
- GitHub Check: Test Blocks Shard 2
- GitHub Check: Build Project
- GitHub Check: Test Blocks Shard 1
- GitHub Check: Test Blocks Shard 3
🔇 Additional comments (7)
packages/server/shared/src/lib/logger/log-cleaner.ts (7)
7-19: LGTM!Good foundational patterns. Using lowercase patterns with substring matching (via
includes) provides flexible case-insensitive detection while keeping the list concise.
21-24: Regex targets JSON string values only.The pattern correctly matches JSON-style
"key": "value"pairs. Note it won't redact URL query parameters or numeric/boolean values with sensitive keys. This is acceptable for typical log contexts where sensitive data appears as string values.
26-29: LGTM!The substring matching approach correctly handles variations like
newPassword,API_TOKEN, andx-authorization-header. Past case-sensitivity concerns are resolved.
31-65: LGTM - robust implementation.Correctly handles circular references with
WeakSet, includes defensive try-catch to prevent logger failures, and processes nested structures recursively. The error fallback provides useful debugging info without compromising the logging flow.
114-128: LGTM - good integration of redaction into the processing pipeline.The placement of
isSensitiveFieldcheck before type-specific handling ensures sensitive keys are fully redacted regardless of value type. The order (truncate → redact for strings, redact → stringify for objects) is correct.
160-177: LGTM - comprehensive error redaction.Stack traces and error messages can inadvertently contain sensitive data. Applying
redactSensitiveDataInStringto these fields andredactSensitiveFieldsto structured params/context ensures thorough sanitization.
180-186: LGTM!The try-catch provides a safety net for edge cases. Since
redactSensitiveFieldsnow handles circular references upstream,JSON.stringifyshould rarely fail, but the defensive error handling is appropriate for logger code.
Fixes OPS-3082
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.