fix(auth): sanitize raw JDBC/SQL exceptions from user-visible error a…#29
fix(auth): sanitize raw JDBC/SQL exceptions from user-visible error a…#29Aarti-panchal01 wants to merge 1 commit into
Conversation
…lerts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds centralized sanitization for backend exception messages and registers it in the application HTTP interceptor chain. Successful and failed HTTP payloads are sanitized while unchanged responses and non-HTTP errors are preserved. ChangesError sanitization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Backend
participant errorSanitizerInterceptor
participant DownstreamHandlers
Backend->>errorSanitizerInterceptor: HTTP response or error payload
errorSanitizerInterceptor->>errorSanitizerInterceptor: Sanitize matched exception message
errorSanitizerInterceptor->>DownstreamHandlers: Original or sanitized payload
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/app-modules/core/http/error-sanitizer.interceptor.ts (2)
85-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid nested ternary operators for better readability.
Consider replacing this nested ternary structure with standard
if/elsestatements. Nested ternaries are notoriously difficult to read and maintain, especially when they span multiple lines.♻️ Proposed refactor
- const sanitizedBody = - typeof err.error === 'string' - ? sanitizeErrorMessage(err.error) !== err.error - ? sanitizeErrorMessage(err.error) - : null - : sanitizeBody(err.error); + let sanitizedBody: Record<string, unknown> | string | null = null; + if (typeof err.error === 'string') { + const sanitizedStr = sanitizeErrorMessage(err.error); + if (sanitizedStr !== err.error) { + sanitizedBody = sanitizedStr; + } + } else { + sanitizedBody = sanitizeBody(err.error); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/app-modules/core/http/error-sanitizer.interceptor.ts` around lines 85 - 90, Refactor the sanitizedBody assignment in the error-sanitization flow to remove the nested ternary, using clear if/else branches for string and non-string err.error values. Preserve the existing behavior: sanitize strings, return the sanitized string only when it changes, otherwise return null, and continue using sanitizeBody for non-string values.
31-40: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueClient-side sanitization does not prevent network-level information disclosure.
While this interceptor successfully hides raw SQL and database exceptions from the user interface and client-side alerts, it does not prevent these errors from being transmitted over the network. A malicious user can still inspect the raw server responses via the browser's Network tab to gather information about the database schema and query structure.
Ensure that these exceptions are also sanitized on the backend so that raw SQL strings never leave the server boundaries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/app-modules/core/http/error-sanitizer.interceptor.ts` around lines 31 - 40, Implement the sanitization at the backend response-generation boundary so raw SQL, database exceptions, and stack details are removed from server error envelopes before transmission; keep the client-side interceptor’s GENERIC_ERROR_MESSAGE as a defense-in-depth fallback, and ensure user-facing responses retain a safe generic message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/app-modules/core/http/error-sanitizer.interceptor.ts`:
- Around line 42-48: Refine the select entry in SERVER_EXCEPTION_PATTERNS so
ordinary instructional messages containing “select” are not sanitized as server
errors. Require SQL-specific context, such as the presence of both SELECT and
FROM, or remove the pattern if the remaining patterns provide adequate coverage;
leave the other patterns unchanged.
---
Nitpick comments:
In `@src/app/app-modules/core/http/error-sanitizer.interceptor.ts`:
- Around line 85-90: Refactor the sanitizedBody assignment in the
error-sanitization flow to remove the nested ternary, using clear if/else
branches for string and non-string err.error values. Preserve the existing
behavior: sanitize strings, return the sanitized string only when it changes,
otherwise return null, and continue using sanitizeBody for non-string values.
- Around line 31-40: Implement the sanitization at the backend
response-generation boundary so raw SQL, database exceptions, and stack details
are removed from server error envelopes before transmission; keep the
client-side interceptor’s GENERIC_ERROR_MESSAGE as a defense-in-depth fallback,
and ensure user-facing responses retain a safe generic message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16ba9ad9-1b58-4b46-9d62-72b3b12e459f
📒 Files selected for processing (2)
src/app/app-modules/core/http/error-sanitizer.interceptor.tssrc/app/app.config.ts
| const SERVER_EXCEPTION_PATTERNS: readonly RegExp[] = [ | ||
| /jdbc/i, | ||
| /\bsql\b/i, | ||
| /\bselect\b/i, | ||
| /exception/i, | ||
| /communications link failure/i, | ||
| ]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Refine the select pattern to avoid false positives in user-facing messages.
The word "select" is highly common in standard validation or instructional messages (e.g., "Please select a valid date", "Select a patient to continue"). The pattern /\bselect\b/i will indiscriminately match these and rewrite helpful user instructions into generic errors.
To prevent this, make the pattern more specific to SQL queries (e.g., checking for both select and from) or rely on the other patterns if they provide sufficient coverage.
🐛 Proposed fix to refine the pattern
const SERVER_EXCEPTION_PATTERNS: readonly RegExp[] = [
/jdbc/i,
/\bsql\b/i,
- /\bselect\b/i,
+ /\bselect\b[\s\S]*?\bfrom\b/i,
/exception/i,
/communications link failure/i,
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const SERVER_EXCEPTION_PATTERNS: readonly RegExp[] = [ | |
| /jdbc/i, | |
| /\bsql\b/i, | |
| /\bselect\b/i, | |
| /exception/i, | |
| /communications link failure/i, | |
| ]; | |
| const SERVER_EXCEPTION_PATTERNS: readonly RegExp[] = [ | |
| /jdbc/i, | |
| /\bsql\b/i, | |
| /\bselect\b[\s\S]*?\bfrom\b/i, | |
| /exception/i, | |
| /communications link failure/i, | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/app-modules/core/http/error-sanitizer.interceptor.ts` around lines 42
- 48, Refine the select entry in SERVER_EXCEPTION_PATTERNS so ordinary
instructional messages containing “select” are not sanitized as server errors.
Require SQL-specific context, such as the presence of both SELECT and FROM, or
remove the pattern if the remaining patterns provide adequate coverage; leave
the other patterns unchanged.
2 participants
fix(auth): sanitize raw JDBC/SQL exceptions from user-visible error alerts
During E2E testing a transient UAT DB outage caused the login screen to
render a raw JDBC/SQL exception verbatim — full SQL query visible to the agent.
Fix: new error-sanitizer.interceptor.ts registered last in the chain.
Rewrites errorMessage in any response body where it matches JDBC, SQL,
select, Exception, or "Communications link failure" to:
"Something went wrong. Please try again or contact support."
Fixes at the single choke point so all ~50 call sites across the app
are covered without touching each one individually.
Note: "select" and "exception" patterns are intentionally broad for
security coverage — flag if any legitimate backend messages are affected.
Summary by CodeRabbit