Skip to content

Conversation

@bitr8
Copy link
Owner

@bitr8 bitr8 commented Jan 2, 2026

Summary

This PR addresses error message information disclosure identified during code review. It introduces a centralized error sanitization utility and updates the global error handler.

Key Changes

File Change
server/utils/errorResponse.ts New utility for safe error responses
server/index.ts Updated global error handler
server/routes/search.ts Use createErrorResponse utility
server/routes/overlayTest.ts Use createErrorResponse utility

Implementation Details

1. Error Sanitization Utility (errorResponse.ts)

Uses a whitelist approach for maximum safety:

// Only messages starting with these patterns are shown in production
const SAFE_MESSAGE_PATTERNS = [
  /^(not found|invalid|missing|required|failed to|unable to|cannot|unauthorized|forbidden)/i,
  /^(no .+ found|.+ is required|.+ not configured)/i,
  /^(connection|network|timeout)/i,
];

Sensitive patterns that are always masked:

  • Stack traces (at Function.xyz (/path/file.ts:123))
  • File system paths (/home/, /var/, C:\)
  • System errors (ENOENT, EACCES, ECONNREFUSED)
  • Credentials (password, secret, token, apikey)
  • Source locations (file.ts:123, node_modules)
  • Database internals (sql, query, table, column)
  • Internal IPs (localhost, 127.0.0.1, 192.168.*, 10.*)

2. Global Error Handler Updates

Before:

res.status(err.status || 500).json({
  message: err.message,  // ⚠️ May contain sensitive data
  errors: err.errors,    // ⚠️ May contain stack traces
});

After:

// Log full details internally
logger.error('Unhandled API error', { label: 'Server', ...err });

// Sanitize for client response
const safeMessage = sanitizeErrorMessage(err.message, 'An unexpected error occurred');
let safeErrors: string[] | undefined;
if (Array.isArray(err.errors)) {
  safeErrors = err.errors.map(e => sanitizeErrorMessage(e, 'Validation error'));
} else if (typeof err.errors === 'string') {
  safeErrors = [sanitizeErrorMessage(err.errors, 'Validation error')];
}
res.status(err.status || 500).json({ message: safeMessage, ...safeErrors });

3. Route-Level Error Handling

Replaced manual error logging with createErrorResponse():

// Before
logger.error('Failed to search Plex', {
  label: 'PlexSearch',
  error: error instanceof Error ? error.message : String(error),
  stack: error instanceof Error ? error.stack : undefined,  // ⚠️ Stack in logs
});
return res.status(500).json({
  error: 'Failed to search Plex',
  message: error instanceof Error ? error.message : String(error),  // ⚠️ Exposed
});

// After
const errorResponse = createErrorResponse(error, 'PlexSearch', 'Failed to search Plex');
return res.status(500).json(errorResponse);  // ✅ Sanitized

Security Analysis

Threat Before After
Stack trace exposure ⚠️ Returned in response ✅ Logged only, masked in response
File path disclosure ⚠️ /var/lib/agregarr/... visible ✅ Replaced with fallback message
Credential leaks ⚠️ Error may contain tokens ✅ Patterns detected and masked
SQL error exposure ⚠️ Query syntax visible ✅ Database patterns masked
Internal IP disclosure ⚠️ 192.168.1.x visible ✅ Private IP ranges masked

Behavior by Environment

Environment NODE_ENV Behavior
Development != 'production' Full error messages shown
Production 'production' Whitelist-only messages
Staging 'staging' Full messages (intentional)

Note: Staging/test environments show full errors for debugging. Set NODE_ENV=production in staging if stricter sanitization needed.


Codex Review Findings (Addressed)

Finding Resolution
Missing logger import in search.ts ✅ Re-added import
Sanitization too permissive ✅ Changed to whitelist approach
err.errors?.map() may throw ✅ Added Array.isArray() check
Non-prod environments leak errors ✅ Documented as intentional

Test Plan

  • Verify error responses in development show full messages
  • Verify error responses in production show only safe messages
  • Test with intentionally thrown errors containing:
    • File paths
    • Stack traces
    • SQL errors
    • Internal IPs
  • Verify internal logs still contain full error details
  • Test OpenAPI validation errors are properly sanitized

Example Error Response

Development (NODE_ENV=development):

{
  "error": "Failed to search Plex",
  "message": "ECONNREFUSED: Connection refused at 192.168.1.178:32400"
}

Production (NODE_ENV=production):

{
  "error": "Failed to search Plex"
}

🤖 Generated with Claude Code

## Changes

1. **Add error response sanitization utility** (server/utils/errorResponse.ts)
   - Uses whitelist approach: only known-safe message patterns shown in production
   - Detects and masks sensitive patterns (stack traces, file paths, credentials, IPs)
   - Shows full errors in development for debugging
   - Includes database, SQL, and internal network patterns
   - Always includes message field for API compatibility

2. **Update global error handler** (server/index.ts)
   - Log full error details internally including stack trace for debugging
   - Return sanitized error messages to clients
   - Safely handle errors array (may be string, array, or undefined)
   - Prevent exposure of internal paths and stack traces

3. **Fix error exposure in routes** (search.ts, overlayTest.ts)
   - Use new createErrorResponse utility for consistent handling
   - Maintain logger import for debug/info logging

## Security Improvements

- Stack traces no longer leaked in API responses
- Internal file paths masked in production
- Credential-related errors sanitized
- System error codes (ENOENT, etc.) hidden from clients
- Database/SQL errors masked
- Internal IP addresses (localhost, private ranges) hidden
- Whitelist approach ensures unknown errors default to safe fallback
@bitr8 bitr8 force-pushed the pr/code-quality-fixes branch from 2998efe to 7ebd63e Compare January 2, 2026 06:25
@bitr8 bitr8 closed this Jan 2, 2026
@bitr8 bitr8 deleted the pr/code-quality-fixes branch January 2, 2026 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants