-
Notifications
You must be signed in to change notification settings - Fork 65
Improve nutrition service error handling for unsupported pets #185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
pet-nutrition-service/server.js
Outdated
Check failure
Code scanning / CodeQL
Missing rate limiting High
a database access
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 17 days ago
The best way to fix this issue is to add a rate limiting middleware to the Express app, so that routes which access the database are protected against excessive requests. The most well-known package for this in the Express ecosystem is express-rate-limit. To address the vulnerability, we should:
- Add
express-rate-limitas a dependency (if not already present). - Import
express-rate-limitinpet-nutrition-service/server.js. - Set up a reasonable limit—for example, 100 requests per 15 minutes per IP, as recommended—via a limiter instance.
- Mount the middleware globally using
app.use(limiter)after other middleware setup lines and before the route handlers.
All changes are within the file pet-nutrition-service/server.js, according to the shown code and requirements. We will only add the import and the rate limiting middleware as described, ensuring existing functionality is unchanged.
-
Copy modified lines R6-R7 -
Copy modified lines R18-R24
| @@ -3,7 +3,8 @@ | ||
| const express = require('express'); | ||
| const mongoose = require('mongoose'); | ||
| const logger = require('pino-http'); | ||
| const NutritionFact = require('./nutrition-fact') | ||
| const NutritionFact = require('./nutrition-fact'); | ||
| const rateLimit = require('express-rate-limit'); | ||
|
|
||
| main().catch(err => console.log(err)); | ||
|
|
||
| @@ -14,6 +15,13 @@ | ||
| app.use(logger()); | ||
| app.use(express.json()); | ||
|
|
||
| // Rate limiter middleware: max 100 requests per 15 minutes per IP | ||
| const limiter = rateLimit({ | ||
| windowMs: 15 * 60 * 1000, // 15 minutes | ||
| max: 100, // limit each IP to 100 requests per windowMs | ||
| }); | ||
| app.use(limiter); | ||
|
|
||
| // GET: Find a NutritionFact by pet_type | ||
| app.get('/nutrition/:pet_type', async (req, res) => { | ||
| try { |
-
Copy modified lines R20-R21
| @@ -17,6 +17,7 @@ | ||
| "ip": "^2.0.1", | ||
| "mongoose": "^8.5.3", | ||
| "pino": "^9.3.2", | ||
| "pino-http": "^10.2.0" | ||
| "pino-http": "^10.2.0", | ||
| "express-rate-limit": "^8.2.0" | ||
| } | ||
| } |
| Package | Version | Security advisories |
| express-rate-limit (npm) | 8.2.0 | None |
Summary
Implements option 2 from the investigation: Improve error handling for unsupported pets with structured responses instead of generic 404s.
Changes
/nutrition/:pet_typeendpoint to return structured error responses for unsupported petsImpact
Error Response Format
{ "error": "Pet type not supported", "message": "Nutrition information not available for 'rabbit'", "pet_type": "rabbit", "supported_pets": ["cat", "dog", "lizard", "snake", "bird", "hamster"], "suggestion": "Try one of these supported pets: cat, dog, lizard, snake, bird, hamster" }This addresses the root cause identified in Application Signals investigation where AI agents receive unhelpful 404 responses for unsupported pet types.