Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,21 @@ const executeScripts = require('./scripts');
res.sendFile(path.join(__dirname, './api-doc/index.html'));
});

/**
* GLOBAL ERROR HANDLER
* Must be registered AFTER all routes
*/
app.use((err, req, res, next) => {
console.error('Global Error Handler:', err);

const status = err.status || err.statusCode || 500;

res.status(status).json({
success: false,
message: err.message || 'Internal Server Error'
});
});
Comment on lines +63 to +72
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potential information disclosure via err.message for uncontrolled errors.

While this handler works well for controlled errors like 'Invalid URL', it exposes err.message directly in the response. From src/middlewares/jsonBodyParserWithErrors.js (lines 8-12), non-JSON parsing errors are passed through next(err) and will reach this handler. Body-parser and other library errors can contain internal details (stack traces, file paths, or implementation specifics) that should not be exposed to clients.

Consider sanitizing the message for non-controlled errors:

🛡️ Proposed fix to prevent information leakage
         app.use((err, req, res, next) => {
             console.error('Global Error Handler:', err);

             const status = err.status || err.statusCode || 500;
+            
+            // Only expose message for known/controlled errors
+            const safeMessage = err.code === 'INVALID_URL' 
+                ? err.message 
+                : (status < 500 ? err.message : 'Internal Server Error');

             res.status(status).json({
                 success: false,
-                message: err.message || 'Internal Server Error'
+                message: safeMessage
             });
         });
📝 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.

Suggested change
app.use((err, req, res, next) => {
console.error('Global Error Handler:', err);
const status = err.status || err.statusCode || 500;
res.status(status).json({
success: false,
message: err.message || 'Internal Server Error'
});
});
app.use((err, req, res, next) => {
console.error('Global Error Handler:', err);
const status = err.status || err.statusCode || 500;
// Only expose message for known/controlled errors
const safeMessage = err.code === 'INVALID_URL'
? err.message
: (status < 500 ? err.message : 'Internal Server Error');
res.status(status).json({
success: false,
message: safeMessage
});
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app.js` around lines 63 - 72, The global error handler (the app.use((err,
req, res, next) => { ... }) block) currently returns err.message directly which
may leak internal info from library errors (see jsonBodyParserWithErrors.js);
change the handler to distinguish controlled/operational errors from unknown
errors (e.g., check a flag like err.isOperational or a safeMessage property set
by jsonBodyParserWithErrors) and only return the original message for controlled
errors; for all other/untrusted errors return a generic message such as
"Internal Server Error" while still logging the full err (console.error or
processLogger) for diagnostics, and update jsonBodyParserWithErrors to mark
parse errors as controlled (set err.isOperational or err.safeMessage) if needed.


// Start the server
app.listen(process.env.APPLICATION_PORT, (err) => {
if (err) {
Expand Down
9 changes: 9 additions & 0 deletions src/middlewares/routeConfigInjector.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ exports.routeConfigInjector = (req, res, next) => {
const routeConfig = routesConfigs.routes.find((route) =>
matchPathsAndExtractParams(route.sourceRoute, urlWithoutQuery)
)

if (!routeConfig) {
const error = new Error('Invalid URL')
Comment thread
Prajwal17Tunerlabs marked this conversation as resolved.
Outdated
error.status = 400
error.code = 'INVALID_URL'

return next(error)
}

if(routeConfig.targetPackages[0] && routeConfig.targetPackages[0].service){
req['baseUrl'] = process.env[`${routeConfig.targetPackages[0].service.toUpperCase()}_SERVICE_BASE_URL`]

Expand Down
8 changes: 8 additions & 0 deletions src/middlewares/targetPackagesInjector.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ exports.targetPackagesInjector = (req, res, next) => {
matchPathsAndExtractParams(route.sourceRoute, urlWithoutQuery)
)

if (!routeConfig) {
const error = new Error('Invalid URL')
error.status = 400
error.code = 'INVALID_URL'

return next(error)
}

// const routeConfig = routesConfigs.routes.find((route) => route.sourceRoute === req.originalUrl)

req['targetPackages'] = routeConfig.targetPackages
Expand Down