diff --git a/.github/workflows/check-hardcoded-urls.yml b/.github/workflows/check-hardcoded-urls.yml index ed621fb..71be434 100644 --- a/.github/workflows/check-hardcoded-urls.yml +++ b/.github/workflows/check-hardcoded-urls.yml @@ -3,6 +3,10 @@ name: Check for hardcoded URLs on: pull_request: +# Read-only: the job only checks out the tree and greps it. +permissions: + contents: read + jobs: hardcoded-urls: runs-on: ubuntu-latest diff --git a/backend/middleware/rateLimit.js b/backend/middleware/rateLimit.js new file mode 100644 index 0000000..1235a7b --- /dev/null +++ b/backend/middleware/rateLimit.js @@ -0,0 +1,21 @@ +const rateLimit = require('express-rate-limit'); + +const WINDOW_MS = 15 * 60 * 1000; // 15 minutes + +const common = { + windowMs: WINDOW_MS, + standardHeaders: true, + legacyHeaders: false, + message: { error: 'Too many requests, please try again later.' }, +}; + +// Broad ceiling for every request the API serves. +const apiLimiter = rateLimit({ ...common, limit: 600 }); + +// Tighter ceiling for routers that write to storage or fan out to other services. +const writeLimiter = rateLimit({ ...common, limit: 60 }); + +// Tightest ceiling for upload routes, the most expensive per request. +const uploadLimiter = rateLimit({ ...common, limit: 20 }); + +module.exports = { apiLimiter, writeLimiter, uploadLimiter }; diff --git a/backend/models/message.js b/backend/models/message.js index 3f0f035..8d9f5c4 100644 --- a/backend/models/message.js +++ b/backend/models/message.js @@ -26,8 +26,18 @@ async function sendMessage( }; if (file && file.data) { - const filename = Date.now() + '_' + file.name; + // file.name arrives from the client, so collapse it to a bare filename and + // drop anything outside [A-Za-z0-9._-]. That strips directory separators, + // '..' segments and NUL bytes, so the write cannot escape UPLOAD_DIR. + const baseName = path.basename(String(file.name ?? '')); + const safeName = baseName.replace(/[^\w.-]/g, '_').replace(/^\.+/, '') || 'upload'; + const filename = Date.now() + '_' + safeName; const filePath = path.join(UPLOAD_DIR, filename); + // Defence in depth: refuse to write if the resolved path is not directly + // inside UPLOAD_DIR. + if (path.dirname(path.resolve(filePath)) !== path.resolve(UPLOAD_DIR)) { + throw new Error('Refusing to write upload outside of the upload directory'); + } fs.writeFileSync(filePath, Buffer.from(file.data, 'base64')); msg.file = { path: '/uploads/' + filename, diff --git a/backend/node_modules/express-rate-limit/index.js b/backend/node_modules/express-rate-limit/index.js new file mode 100644 index 0000000..7134246 --- /dev/null +++ b/backend/node_modules/express-rate-limit/index.js @@ -0,0 +1,13 @@ +// Minimal stand-in for express-rate-limit, mirroring the express, aws-sdk and +// stripe stubs already tracked under backend/node_modules so that `npm test` +// runs without a full install. Counts nothing; real limiting comes from the +// published package once dependencies are installed. +function rateLimit() { + return function rateLimitMiddleware(_req, _res, next) { + if (typeof next === 'function') next(); + }; +} + +module.exports = rateLimit; +module.exports.default = rateLimit; +module.exports.rateLimit = rateLimit; diff --git a/backend/package-lock.json b/backend/package-lock.json index 6969d71..df0468b 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -16,6 +16,7 @@ "cors": "^2.8.5", "dotenv": "^16.4.7", "express": "^4.21.2", + "express-rate-limit": "^7.5.1", "franc": "^6.1.0", "jsonwebtoken": "^9.0.2", "openai": "^4.76.0", @@ -2861,6 +2862,7 @@ "resolved": "https://registry.npmjs.org/express/-/express-4.22.2.tgz", "integrity": "sha512-IuL+Elrou2ZvCFHs18/CIzy2Nzvo25nZ1/D2eIZlz7c+QUayAcYoiM2BthCjs+EBHVpjYjcuLDAiCWgeIX3X1Q==", "license": "MIT", + "peer": true, "dependencies": { "accepts": "~1.3.8", "array-flatten": "1.1.1", @@ -2902,6 +2904,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", diff --git a/backend/package.json b/backend/package.json index 6db3cbb..da4331d 100644 --- a/backend/package.json +++ b/backend/package.json @@ -15,18 +15,19 @@ "cors": "^2.8.5", "dotenv": "^16.4.7", "express": "^4.21.2", + "express-rate-limit": "^7.5.1", "franc": "^6.1.0", "jsonwebtoken": "^9.0.2", - "twilio": "^4.21.0", - "openai": "^4.76.0" + "openai": "^4.76.0", + "twilio": "^4.21.0" }, "devDependencies": { "aws-sdk": "^2.1534.0", "jest": "^29.7.0" }, "overrides": { - "qs": "^6.14.1", - "jws": "^3.2.3" + "qs": "^6.14.1", + "jws": "^3.2.3" }, "jest": { "testEnvironment": "node" diff --git a/backend/routes/agrotourismRoutes.js b/backend/routes/agrotourismRoutes.js index 35c6bf6..5d23638 100644 --- a/backend/routes/agrotourismRoutes.js +++ b/backend/routes/agrotourismRoutes.js @@ -4,6 +4,7 @@ const multer = require("multer"); const path = require("path"); const Agrotourism = require("../models/agrotourism"); const authMiddleware = require("../middleware/authMiddleware"); +const { apiLimiter, uploadLimiter } = require("../middleware/rateLimit"); // Configure Multer for image uploads const storage = multer.diskStorage({ @@ -16,6 +17,10 @@ const storage = multer.diskStorage({ }); const upload = multer({ storage }); +// Listing reads hit Mongo; /create also accepts up to five image uploads. +router.use(apiLimiter); +router.use("/create", uploadLimiter); + // Create Agrotourism Listing router.post("/create", authMiddleware, upload.array("images", 5), async (req, res) => { try { diff --git a/backend/routes/communicationRoutes.js b/backend/routes/communicationRoutes.js index 3a0367c..d7d2928 100644 --- a/backend/routes/communicationRoutes.js +++ b/backend/routes/communicationRoutes.js @@ -3,8 +3,10 @@ const router = express.Router(); const commController = require('../controllers/communication_controller'); const auth = require('../middleware/authMiddleware'); const asyncHandler = require('../utils/asyncHandler'); +const { writeLimiter } = require('../middleware/rateLimit'); router.use(auth); +router.use(writeLimiter); router.post('/:conversationId', asyncHandler(commController.sendMessage)); router.get('/:conversationId', asyncHandler(commController.listMessages)); diff --git a/backend/routes/conversationRoutes.js b/backend/routes/conversationRoutes.js index 4973900..c33161f 100644 --- a/backend/routes/conversationRoutes.js +++ b/backend/routes/conversationRoutes.js @@ -3,8 +3,10 @@ const router = express.Router(); const auth = require('../middleware/authMiddleware'); const ctrl = require('../controllers/conversation_controller'); const asyncHandler = require('../utils/asyncHandler'); +const { writeLimiter } = require('../middleware/rateLimit'); router.use(auth); +router.use(writeLimiter); router.post('/', asyncHandler(ctrl.create)); router.get('/', asyncHandler(ctrl.list)); router.put('/:id', asyncHandler(ctrl.rename)); diff --git a/backend/routes/depositRoutes.js b/backend/routes/depositRoutes.js index a75085a..1ef676f 100644 --- a/backend/routes/depositRoutes.js +++ b/backend/routes/depositRoutes.js @@ -3,9 +3,12 @@ const router = express.Router(); const authMiddleware = require('../middleware/authMiddleware'); const depositController = require('../controllers/deposit_controller'); const asyncHandler = require('../utils/asyncHandler'); +const { writeLimiter } = require('../middleware/rateLimit'); -// All routes are protected +// All routes are protected and rate limited: every handler moves funds +// or reads transaction history. router.use(authMiddleware); +router.use(writeLimiter); // Get or create user deposit account router.get('/', asyncHandler(depositController.getOrCreateAccount)); diff --git a/backend/routes/logRoutes.js b/backend/routes/logRoutes.js index 3bb0eef..8d51607 100644 --- a/backend/routes/logRoutes.js +++ b/backend/routes/logRoutes.js @@ -6,6 +6,10 @@ const { createTransactionLogItem } = require('../models/transactionLog'); const authMiddleware = require('../middleware/authMiddleware'); +const { writeLimiter } = require('../middleware/rateLimit'); + +// Both handlers touch DynamoDB, so cap request volume per client. +router.use(writeLimiter); // Store a new transaction log entry router.post('/logs', authMiddleware, async (req, res) => { diff --git a/backend/server.js b/backend/server.js index 6623eac..c561e73 100644 --- a/backend/server.js +++ b/backend/server.js @@ -14,6 +14,7 @@ try { } const path = require("path"); const authMiddleware = require("./middleware/authMiddleware"); +const { apiLimiter } = require("./middleware/rateLimit"); // Load environment variables dotenv.config(); @@ -129,6 +130,8 @@ global.emitToken = emitToken; global.emitMessage = emitMessage; // Middleware +// Global ceiling ahead of auth so unauthenticated floods are shed early. +app.use(apiLimiter); app.use(authMiddleware); app.use('/uploads', express.static(path.join(__dirname, 'uploads')));