🛡️ Sentinel: [MEDIUM] Fix Information Disclosure in Error Handling#66
Conversation
- Prevent Information Disclosure by sanitizing generic error messages. - Updated toErrorPayload to map raw Javascript Exception strings to generic fallback messages before sending to external API clients. - Updated relevant unit tests and added critical learning to sentinel journal. Co-authored-by: mbayue <[email protected]>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant Client as External Client
participant API as API Route
participant Controller as Controller / Service
participant ErrorHandler as Error Handler Middleware
participant ErrGen as ErrorPayloadGenerator (toErrorPayload)
Client->>API: HTTP Request
API->>Controller: Invoke business logic
Controller--xController: Throws Error (e.g., "Rate limit 429", "private repo", "timeout")
Controller-->>ErrorHandler: Uncaught exception
ErrorHandler->>ErrGen: toErrorPayload(err)
Note over ErrGen: NEW: Sanitize generic error messages<br/>(raw msg may contain sensitive info)
alt Rate limit error (msg includes "rate limit" or "429")
ErrGen->>ErrGen: Determine: isRate = true
ErrGen->>ErrGen: Set sanitizedError = "Rate limit exceeded"
ErrGen-->>ErrorHandler: { error: "Rate limit exceeded", code: "RATE_LIMIT_EXCEEDED", status: 429, retryable: true }
else Private / Not Found error (msg includes "private", "not found", "404")
ErrGen->>ErrGen: Determine: isPrivate = true
ErrGen->>ErrGen: Set sanitizedError = "Repository not found or private"
ErrGen-->>ErrorHandler: { error: "Repository not found or private", code: "REPO_INACCESSIBLE", status: 404, retryable: false }
else Generic / other error (including timeout, network)
ErrGen->>ErrGen: Neither rate nor private
ErrGen->>ErrGen: Set sanitizedError = "Internal Server Error"
ErrGen-->>ErrorHandler: { error: "Internal Server Error", code: "INTERNAL_ERROR", status: 500, retryable: true (if timeout/network) }
end
ErrorHandler-->>API: JSON response with sanitized payload
API-->>Client: HTTP Response (status 429/404/500)
Note over Client,API: Previously the raw `err.message` (e.g., "Rate limit 429", "timeout") was returned as `error` property,<br/>potentially leaking internal details.
🚨 Severity: MEDIUM
💡 Vulnerability: The error payload generator (
server/utils/errors.ts) was returning raw JavascriptError.messagestrings directly to the client as theerrorproperty when an unknown exception occurred.🎯 Impact: Passing raw error messages from uncaught exceptions to external API clients can leak sensitive internal information (e.g., file paths, internal IP addresses, database schemas, API keys in URLs) to potential attackers. This allows for reconnaissance and targeted attacks.
🔧 Fix: Sanitized error payloads to predefined, generic fallback messages (e.g., "Internal Server Error", "Rate limit exceeded") before returning them in an HTTP response.
✅ Verification: Ran
bun testand verified tests inserver/utils/errors.test.tspass and linter doesn't show errors on modified files.PR created automatically by Jules for task 7355982610702721678 started by @mbayue
Summary by cubic
Prevents information disclosure by sanitizing error payloads sent to clients. Maps raw exception messages to safe, generic messages while keeping appropriate status codes and retry behavior.
toErrorPayload: map rate-limit to "Rate limit exceeded", private/not-found to "Repository not found or private", else "Internal Server Error".RATE_LIMIT_EXCEEDED429,REPO_INACCESSIBLE404,INTERNAL_ERROR500; retryable for rate, timeout, or network errors.server/utils/errors.test.tsand documented the fix in.jules/sentinel.md.Written for commit 87bf423. Summary will update on new commits.