Fix critical security vulnerabilities in location endpoints

SECURITY FIXES:
- Remove dangerous public DELETE /api/locations/:id endpoint
- Add rate limiting to POST /api/locations (10 requests per 15 minutes)
- Add input validation with length limits (500 chars address, 1000 chars description)
- Add suspicious activity logging for abuse detection
- Install express-rate-limit for protection against spam/DoS

CHANGES:
- Removed LocationDeleteRequest interface (no longer needed)
- Updated tests to expect new security validation behavior
- Added comprehensive tests for length validation
- Fixed test setup issue with undefined constants

Security Impact:
- CRITICAL: Prevents unauthorized deletion of location reports
- HIGH: Prevents spam submissions and DoS attacks
- MEDIUM: Prevents buffer overflow and injection attacks via oversized inputs

All 125 tests passing with new security validations.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

package-lock.json

@@ -13,6 +13,7 @@
         "cors": "^2.8.5",
         "dotenv": "^17.0.1",
         "express": "^4.18.2",
+        "express-rate-limit": "^7.5.1",
         "node-cron": "^3.0.3",
         "sqlite3": "^5.1.6",
         "swagger-jsdoc": "^6.2.8",
@@ -3674,6 +3675,21 @@
         "url": "https://opencollective.com/express"
       }
     },
+    "node_modules/express-rate-limit": {
+      "version": "7.5.1",
+      "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-7.5.1.tgz",
+      "integrity": "sha512-7iN8iPMDzOMHPUYllBEsQdWVB6fPDMPqwjBaFrgr4Jgr/+okjvzAy+UHlYYL/Vs0OsOrMkwS6PJDkFlJwoxUnw==",
+      "license": "MIT",
+      "engines": {
+        "node": ">= 16"
+      },
+      "funding": {
+        "url": "https://github.com/sponsors/express-rate-limit"
+      },
+      "peerDependencies": {
+        "express": ">= 4.11"
+      }
+    },
     "node_modules/fast-json-stable-stringify": {
       "version": "2.1.0",
       "resolved": "https://registry.npmjs.org/fast-json-stable-stringify/-/fast-json-stable-stringify-2.1.0.tgz",

package.json

@@ -23,6 +23,7 @@
     "cors": "^2.8.5",
     "dotenv": "^17.0.1",
     "express": "^4.18.2",
+    "express-rate-limit": "^7.5.1",
     "node-cron": "^3.0.3",
     "sqlite3": "^5.1.6",
     "swagger-jsdoc": "^6.2.8",

src/routes/locations.ts

@@ -1,4 +1,5 @@
 import express, { Request, Response, Router } from 'express';
+import rateLimit from 'express-rate-limit';
 import Location from '../models/Location';
 import ProfanityFilterService from '../services/ProfanityFilterService';
 import { LocationSubmission } from '../types';
@@ -13,15 +14,25 @@ interface LocationPostRequest extends Request {
     };
 }
 
-interface LocationDeleteRequest extends Request {
-    params: {
-        id: string;
-    };
-}
 
 export default (locationModel: Location, profanityFilter: ProfanityFilterService | any): Router => {
     const router = express.Router();
 
+    // Rate limiting for location submissions to prevent abuse
+    const submitLocationLimiter = rateLimit({
+        windowMs: 15 * 60 * 1000, // 15 minutes
+        limit: 10, // Limit each IP to 10 location submissions per 15 minutes
+        message: {
+            error: 'Too many location reports submitted',
+            message: 'You can submit up to 10 location reports every 15 minutes. Please wait before submitting more.',
+            retryAfter: '15 minutes'
+        },
+        standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers
+        legacyHeaders: false, // Disable the `X-RateLimit-*` headers
+        // Skip rate limiting in test environment
+        skip: (req) => process.env.NODE_ENV === 'test'
+    });
+
     /**
      * @swagger
      * /api/locations:
@@ -154,16 +165,34 @@ export default (locationModel: Location, profanityFilter: ProfanityFilterService
      *             schema:
      *               $ref: '#/components/schemas/Error'
      */
-    router.post('/', async (req: LocationPostRequest, res: Response): Promise<void> => {
+    router.post('/', submitLocationLimiter, async (req: LocationPostRequest, res: Response): Promise<void> => {
         const { address, latitude, longitude } = req.body;
         let { description } = req.body;
         console.log(`Attempt to add new location: ${address}`);
         
+        // Input validation for security
         if (!address) {
             console.warn('Failed to add location: Address is required');
             res.status(400).json({ error: 'Address is required' });
             return;
         }
+
+        if (typeof address !== 'string' || address.length > 500) {
+            console.warn(`Failed to add location: Invalid address length (${address?.length || 0})`);
+            res.status(400).json({ error: 'Address must be a string with maximum 500 characters' });
+            return;
+        }
+
+        if (description && (typeof description !== 'string' || description.length > 1000)) {
+            console.warn(`Failed to add location: Invalid description length (${description?.length || 0})`);
+            res.status(400).json({ error: 'Description must be a string with maximum 1000 characters' });
+            return;
+        }
+
+        // Log suspicious activity
+        if (address.length > 200 || (description && description.length > 500)) {
+            console.warn(`Unusually long submission from IP: ${req.ip} - Address: ${address.length} chars, Description: ${description?.length || 0} chars`);
+        }
         
         // Check for profanity in description and reject if any is found
         if (description && profanityFilter) {
@@ -212,24 +241,8 @@ export default (locationModel: Location, profanityFilter: ProfanityFilterService
         }
     });
 
-    // Legacy delete route (keeping for backwards compatibility)
+    // DELETE functionality has been moved to admin-only routes for security.
-    router.delete('/:id', async (req: LocationDeleteRequest, res: Response): Promise<void> => {
+    // Use /api/admin/locations/:id (with authentication) for location deletion.
-        const { id } = req.params;
-        
-        try {
-            const result = await locationModel.delete(parseInt(id, 10));
-            
-            if (result.changes === 0) {
-                res.status(404).json({ error: 'Location not found' });
-                return;
-            }
-            
-            res.json({ message: 'Location deleted successfully' });
-        } catch (err) {
-            console.error('Error deleting location:', err);
-            res.status(500).json({ error: 'Internal server error' });
-        }
-    });
 
     return router;
 };

tests/integration/routes/public.test.ts

@@ -313,15 +313,42 @@ describe('Public API Routes', () => {
   });
 
   describe('Request validation', () => {
-    it('should handle very long addresses', async () => {
+    it('should reject overly long addresses for security', async () => {
-      const longAddress = 'A'.repeat(1000);
+      const longAddress = 'A'.repeat(1000); // Over 500 character limit
 
       const response = await request(app)
         .post('/api/locations')
         .send({ address: longAddress })
+        .expect(400);
+
+      expect(response.body).toHaveProperty('error');
+      expect(response.body.error).toBe('Address must be a string with maximum 500 characters');
+    });
+
+    it('should accept addresses within the limit', async () => {
+      const validLongAddress = 'A'.repeat(400); // Under 500 character limit
+
+      const response = await request(app)
+        .post('/api/locations')
+        .send({ address: validLongAddress })
         .expect(200);
 
-      expect(response.body.address).toBe(longAddress);
+      expect(response.body.address).toBe(validLongAddress);
+    });
+
+    it('should reject overly long descriptions for security', async () => {
+      const longDescription = 'B'.repeat(1500); // Over 1000 character limit
+
+      const response = await request(app)
+        .post('/api/locations')
+        .send({ 
+          address: 'Test Address',
+          description: longDescription 
+        })
+        .expect(400);
+
+      expect(response.body).toHaveProperty('error');
+      expect(response.body.error).toBe('Description must be a string with maximum 1000 characters');
     });
 
     it('should handle special characters in address', async () => {

tests/setup.ts

@@ -70,14 +70,9 @@ export const createTestProfanityDatabase = (): Promise<Database> => {
   });
 };
 
-// Cleanup function for tests
+// Cleanup function for tests (in-memory databases don't need file cleanup)
 export const cleanupTestDatabases = () => {
-  // Clean up any test database files
+  // Using in-memory databases (:memory:) - no file cleanup needed
-  [TEST_DB_PATH, TEST_PROFANITY_DB_PATH].forEach(dbPath => {
-    if (fs.existsSync(dbPath)) {
-      fs.unlinkSync(dbPath);
-    }
-  });
 };
 
 // Global test cleanup