Conversation
Co-authored-by: johnmeshulam <johnmeshulam@users.noreply.github.com>
Co-authored-by: johnmeshulam <johnmeshulam@users.noreply.github.com>
|
|
||
| router.use( | ||
| '/:eventId', | ||
| adminAuth, // This integration is managed from the admin panel, by admin users. |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
In general, the fix is to add a rate-limiting middleware to the affected routes, using a standard library such as express-rate-limit. This middleware will cap how many requests a given client (e.g., IP address) can make within a time window, reducing the risk that repeated CSV uploads or other heavy operations overwhelm the server or database.
For this specific file (apps/backend/src/routers/integrations/sendgrid/index.ts), the best minimal change is:
- Import
express-rate-limit. - Create a small limiter instance configured for admin usage (e.g., a lowish
maxper 15-minute or per-minute window). - Apply that limiter as a middleware on the
/\:eventIdsubrouter before the heavy routes, so both/upload-contactsand/send-testare protected, without altering their existing logic, auth, or permissions.
Concretely:
-
Add an import at the top:
import rateLimit from 'express-rate-limit';
-
Just after creating the
router, define anadminLimiter:const adminLimiter = rateLimit({ windowMs: 15 * 60 * 1000, // 15 minutes max: 100, // limit each IP to 100 requests per windowMs for these routes standardHeaders: true, legacyHeaders: false });
-
Extend the
router.use('/:eventId', ...)chain to include this limiter (after auth, so errors will not leak details to unauthenticated users, but before heavy work is done):router.use( '/:eventId', adminAuth, adminLimiter, attachEvent(), requirePermission('MANAGE_EVENT_DETAILS') );
This keeps the integration behavior the same for legitimate admins (except for now hitting the configured rate limit if they send excessive requests) and satisfies CodeQL’s concern by explicitly rate-limiting the expensive, authorized handlers.
| @@ -8,12 +8,21 @@ | ||
| import { sendEmailWithSendGrid } from './sendgrid-lib'; | ||
| import { generatePlaceholderPDF } from './placeholder-generator'; | ||
| import { CSVRecord } from './types'; | ||
| import rateLimit from 'express-rate-limit'; | ||
|
|
||
| const router = express.Router({ mergeParams: true }); | ||
|
|
||
| const adminLimiter = rateLimit({ | ||
| windowMs: 15 * 60 * 1000, // 15 minutes | ||
| max: 100, // limit each IP to 100 requests per window for admin integration actions | ||
| standardHeaders: true, | ||
| legacyHeaders: false | ||
| }); | ||
|
|
||
| router.use( | ||
| '/:eventId', | ||
| adminAuth, // This integration is managed from the admin panel, by admin users. | ||
| adminLimiter, | ||
| attachEvent(), | ||
| requirePermission('MANAGE_EVENT_DETAILS') | ||
| ); |
Description
Closes #1656