s3 image file validation

2025-12-12 13:33:24 -05:00
parent 763945fef4
commit 1dee5232a0
20 changed files with 657 additions and 654 deletions
--- a/backend/routes/conditionChecks.js
+++ b/backend/routes/conditionChecks.js
@@ -2,6 +2,8 @@ const express = require("express");
 const { authenticateToken } = require("../middleware/auth");
 const ConditionCheckService = require("../services/conditionCheckService");
 const logger = require("../utils/logger");
+const { validateS3Keys } = require("../utils/s3KeyValidator");
+const { IMAGE_LIMITS } = require("../config/imageLimits");

 const router = express.Router();

@@ -13,10 +15,24 @@ router.post("/:rentalId", authenticateToken, async (req, res) => {
    const userId = req.user.id;

    // Ensure imageFilenames is an array (S3 keys)
-    const imageFilenames = Array.isArray(rawImageFilenames)
+    const imageFilenamesArray = Array.isArray(rawImageFilenames)
      ? rawImageFilenames
      : [];

+    // Validate S3 keys format and folder
+    const keyValidation = validateS3Keys(imageFilenamesArray, "condition-checks", {
+      maxKeys: IMAGE_LIMITS.conditionChecks,
+    });
+    if (!keyValidation.valid) {
+      return res.status(400).json({
+        success: false,
+        error: keyValidation.error,
+        details: keyValidation.invalidKeys,
+      });
+    }
+
+    const imageFilenames = imageFilenamesArray;
+
    const conditionCheck = await ConditionCheckService.submitConditionCheck(
      rentalId,
      checkType,
--- a/backend/routes/forum.js
+++ b/backend/routes/forum.js
@@ -6,6 +6,8 @@ const logger = require('../utils/logger');
 const emailServices = require('../services/email');
 const googleMapsService = require('../services/googleMapsService');
 const locationService = require('../services/locationService');
+const { validateS3Keys } = require('../utils/s3KeyValidator');
+const { IMAGE_LIMITS } = require('../config/imageLimits');
 const router = express.Router();

 // Helper function to build nested comment tree
@@ -239,10 +241,20 @@ router.get('/posts/:id', optionalAuth, async (req, res, next) => {
 // POST /api/forum/posts - Create new post
 router.post('/posts', authenticateToken, async (req, res, next) => {
  try {
-    let { title, content, category, tags, zipCode, latitude: providedLat, longitude: providedLng, imageFilenames } = req.body;
+    let { title, content, category, tags, zipCode, latitude: providedLat, longitude: providedLng, imageFilenames: rawImageFilenames } = req.body;

-    // Ensure imageFilenames is an array
-    imageFilenames = Array.isArray(imageFilenames) ? imageFilenames : [];
+    // Ensure imageFilenames is an array and validate S3 keys
+    const imageFilenamesArray = Array.isArray(rawImageFilenames) ? rawImageFilenames : [];
+
+    const keyValidation = validateS3Keys(imageFilenamesArray, 'forum', { maxKeys: IMAGE_LIMITS.forum });
+    if (!keyValidation.valid) {
+      return res.status(400).json({
+        error: keyValidation.error,
+        details: keyValidation.invalidKeys
+      });
+    }
+
+    const imageFilenames = imageFilenamesArray;

    // Initialize location fields
    let latitude = null;
@@ -488,9 +500,26 @@ router.put('/posts/:id', authenticateToken, async (req, res, next) => {
      return res.status(403).json({ error: 'Unauthorized' });
    }

-    const { title, content, category, tags } = req.body;
+    const { title, content, category, tags, imageFilenames: rawImageFilenames } = req.body;

-    await post.update({ title, content, category });
+    // Build update object
+    const updateData = { title, content, category };
+
+    // Handle imageFilenames if provided
+    if (rawImageFilenames !== undefined) {
+      const imageFilenamesArray = Array.isArray(rawImageFilenames) ? rawImageFilenames : [];
+
+      const keyValidation = validateS3Keys(imageFilenamesArray, 'forum', { maxKeys: IMAGE_LIMITS.forum });
+      if (!keyValidation.valid) {
+        return res.status(400).json({
+          error: keyValidation.error,
+          details: keyValidation.invalidKeys
+        });
+      }
+      updateData.imageFilenames = imageFilenamesArray;
+    }
+
+    await post.update(updateData);

    // Update tags if provided
    if (tags !== undefined) {
@@ -927,8 +956,18 @@ router.post('/posts/:id/comments', authenticateToken, async (req, res, next) =>
      }
    }

-    // Ensure imageFilenames is an array
-    const imageFilenames = Array.isArray(rawImageFilenames) ? rawImageFilenames : [];
+    // Ensure imageFilenames is an array and validate S3 keys
+    const imageFilenamesArray = Array.isArray(rawImageFilenames) ? rawImageFilenames : [];
+
+    const keyValidation = validateS3Keys(imageFilenamesArray, 'forum', { maxKeys: IMAGE_LIMITS.forum });
+    if (!keyValidation.valid) {
+      return res.status(400).json({
+        error: keyValidation.error,
+        details: keyValidation.invalidKeys
+      });
+    }
+
+    const imageFilenames = imageFilenamesArray;

    const comment = await ForumComment.create({
      postId: req.params.id,
--- a/backend/routes/items.js
+++ b/backend/routes/items.js
@@ -3,8 +3,56 @@ const { Op } = require("sequelize");
 const { Item, User, Rental } = require("../models"); // Import from models/index.js to get models with associations
 const { authenticateToken, requireVerifiedEmail, requireAdmin, optionalAuth } = require("../middleware/auth");
 const logger = require("../utils/logger");
+const { validateS3Keys } = require("../utils/s3KeyValidator");
+const { IMAGE_LIMITS } = require("../config/imageLimits");
 const router = express.Router();

+// Allowed fields for item create/update (prevents mass assignment)
+const ALLOWED_ITEM_FIELDS = [
+  'name',
+  'description',
+  'pickUpAvailable',
+  'localDeliveryAvailable',
+  'localDeliveryRadius',
+  'shippingAvailable',
+  'inPlaceUseAvailable',
+  'pricePerHour',
+  'pricePerDay',
+  'pricePerWeek',
+  'pricePerMonth',
+  'replacementCost',
+  'address1',
+  'address2',
+  'city',
+  'state',
+  'zipCode',
+  'country',
+  'latitude',
+  'longitude',
+  'imageFilenames',
+  'isAvailable',
+  'rules',
+  'availableAfter',
+  'availableBefore',
+  'specifyTimesPerDay',
+  'weeklyTimes',
+];
+
+/**
+ * Extract only allowed fields from request body
+ * @param {Object} body - Request body
+ * @returns {Object} - Object with only allowed fields
+ */
+function extractAllowedFields(body) {
+  const result = {};
+  for (const field of ALLOWED_ITEM_FIELDS) {
+    if (body[field] !== undefined) {
+      result[field] = body[field];
+    }
+  }
+  return result;
+}
+
 router.get("/", async (req, res, next) => {
  try {
    const {
@@ -232,8 +280,27 @@ router.get("/:id", optionalAuth, async (req, res, next) => {

 router.post("/", authenticateToken, requireVerifiedEmail, async (req, res, next) => {
  try {
+    // Extract only allowed fields (prevents mass assignment)
+    const allowedData = extractAllowedFields(req.body);
+
+    // Validate imageFilenames if provided
+    if (allowedData.imageFilenames) {
+      const imageFilenames = Array.isArray(allowedData.imageFilenames)
+        ? allowedData.imageFilenames
+        : [];
+
+      const keyValidation = validateS3Keys(imageFilenames, 'items', { maxKeys: IMAGE_LIMITS.items });
+      if (!keyValidation.valid) {
+        return res.status(400).json({
+          error: keyValidation.error,
+          details: keyValidation.invalidKeys
+        });
+      }
+      allowedData.imageFilenames = imageFilenames;
+    }
+
    const item = await Item.create({
-      ...req.body,
+      ...allowedData,
      ownerId: req.user.id,
    });

@@ -300,7 +367,26 @@ router.put("/:id", authenticateToken, async (req, res, next) => {
      return res.status(403).json({ error: "Unauthorized" });
    }

-    await item.update(req.body);
+    // Extract only allowed fields (prevents mass assignment)
+    const allowedData = extractAllowedFields(req.body);
+
+    // Validate imageFilenames if provided
+    if (allowedData.imageFilenames !== undefined) {
+      const imageFilenames = Array.isArray(allowedData.imageFilenames)
+        ? allowedData.imageFilenames
+        : [];
+
+      const keyValidation = validateS3Keys(imageFilenames, 'items', { maxKeys: IMAGE_LIMITS.items });
+      if (!keyValidation.valid) {
+        return res.status(400).json({
+          error: keyValidation.error,
+          details: keyValidation.invalidKeys
+        });
+      }
+      allowedData.imageFilenames = imageFilenames;
+    }
+
+    await item.update(allowedData);

    const updatedItem = await Item.findByPk(item.id, {
      include: [
--- a/backend/routes/messages.js
+++ b/backend/routes/messages.js
@@ -8,6 +8,8 @@ const { Op } = require('sequelize');
 const emailServices = require('../services/email');
 const fs = require('fs');
 const path = require('path');
+const { validateS3Keys } = require('../utils/s3KeyValidator');
+const { IMAGE_LIMITS } = require('../config/imageLimits');
 const router = express.Router();

 // Get all messages for the current user (inbox)
@@ -240,6 +242,17 @@ router.post('/', authenticateToken, async (req, res, next) => {
  try {
    const { receiverId, content, imageFilename } = req.body;

+    // Validate imageFilename if provided
+    if (imageFilename) {
+      const keyValidation = validateS3Keys([imageFilename], 'messages', { maxKeys: IMAGE_LIMITS.messages });
+      if (!keyValidation.valid) {
+        return res.status(400).json({
+          error: keyValidation.error,
+          details: keyValidation.invalidKeys
+        });
+      }
+    }
+
    // Check if receiver exists
    const receiver = await User.findByPk(receiverId);
    if (!receiver) {
--- a/backend/routes/users.js
+++ b/backend/routes/users.js
@@ -1,13 +1,41 @@
 const express = require('express');
 const { User, UserAddress } = require('../models'); // Import from models/index.js to get models with associations
 const { authenticateToken } = require('../middleware/auth');
-const { uploadProfileImage } = require('../middleware/upload');
 const logger = require('../utils/logger');
 const userService = require('../services/UserService');
-const fs = require('fs').promises;
-const path = require('path');
+const { validateS3Keys } = require('../utils/s3KeyValidator');
+const { IMAGE_LIMITS } = require('../config/imageLimits');
 const router = express.Router();

+// Allowed fields for profile update (prevents mass assignment)
+const ALLOWED_PROFILE_FIELDS = [
+  'firstName',
+  'lastName',
+  'email',
+  'phone',
+  'address1',
+  'address2',
+  'city',
+  'state',
+  'zipCode',
+  'country',
+  'imageFilename',
+  'itemRequestNotificationRadius',
+];
+
+/**
+ * Extract only allowed fields from request body
+ */
+function extractAllowedProfileFields(body) {
+  const result = {};
+  for (const field of ALLOWED_PROFILE_FIELDS) {
+    if (body[field] !== undefined) {
+      result[field] = body[field];
+    }
+  }
+  return result;
+}
+
 router.get('/profile', authenticateToken, async (req, res, next) => {
  try {
    const user = await User.findByPk(req.user.id, {
@@ -182,8 +210,22 @@ router.get('/:id', async (req, res, next) => {

 router.put('/profile', authenticateToken, async (req, res, next) => {
  try {
+    // Extract only allowed fields (prevents mass assignment)
+    const allowedData = extractAllowedProfileFields(req.body);
+
+    // Validate imageFilename if provided
+    if (allowedData.imageFilename !== undefined && allowedData.imageFilename !== null) {
+      const keyValidation = validateS3Keys([allowedData.imageFilename], 'profiles', { maxKeys: IMAGE_LIMITS.profile });
+      if (!keyValidation.valid) {
+        return res.status(400).json({
+          error: keyValidation.error,
+          details: keyValidation.invalidKeys
+        });
+      }
+    }
+
    // Use UserService to handle update and email notification
-    const updatedUser = await userService.updateProfile(req.user.id, req.body);
+    const updatedUser = await userService.updateProfile(req.user.id, allowedData);

    res.json(updatedUser);
  } catch (error) {
@@ -192,65 +234,4 @@ router.put('/profile', authenticateToken, async (req, res, next) => {
  }
 });

-// Upload profile image endpoint
-router.post('/profile/image', authenticateToken, (req, res) => {
-  uploadProfileImage(req, res, async (err) => {
-    if (err) {
-      const reqLogger = logger.withRequestId(req.id);
-      reqLogger.error("Profile image upload error", {
-        error: err.message,
-        userId: req.user.id
-      });
-      return res.status(400).json({ error: err.message });
-    }
-
-    if (!req.file) {
-      return res.status(400).json({ error: 'No file uploaded' });
-    }
-
-    try {
-      // Delete old profile image if exists
-      const user = await User.findByPk(req.user.id);
-      if (user.imageFilename) {
-        const oldImagePath = path.join(__dirname, '../uploads/profiles', user.imageFilename);
-        try {
-          await fs.unlink(oldImagePath);
-        } catch (unlinkErr) {
-          const reqLogger = logger.withRequestId(req.id);
-          reqLogger.warn("Error deleting old profile image", {
-            error: unlinkErr.message,
-            userId: req.user.id,
-            oldImagePath
-          });
-        }
-      }
-
-      // Update user with new image filename
-      await user.update({
-        imageFilename: req.file.filename
-      });
-
-      const reqLogger = logger.withRequestId(req.id);
-      reqLogger.info("Profile image uploaded successfully", {
-        userId: req.user.id,
-        filename: req.file.filename
-      });
-
-      res.json({
-        message: 'Profile image uploaded successfully',
-        filename: req.file.filename,
-        imageUrl: `/uploads/profiles/${req.file.filename}`
-      });
-    } catch (error) {
-      const reqLogger = logger.withRequestId(req.id);
-      reqLogger.error("Profile image database update failed", {
-        error: error.message,
-        stack: error.stack,
-        userId: req.user.id
-      });
-      res.status(500).json({ error: 'Failed to update profile image' });
-    }
-  });
-});
-
 module.exports = router;