Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions backend/src/middleware/validation.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { body, param, query, validationResult } from 'express-validator';
import { Request, Response, NextFunction } from 'express';
import mongoose from 'mongoose';
import { sanitizeRegexInput as securitySanitizeRegex } from '../utils/security';

// Custom validation functions
const isValidObjectId = (value: string) => {
Expand All @@ -11,6 +12,9 @@ const isValidProjectId = (value: string) => {
return value === 'global' || value === 'default' || isValidObjectId(value);
};

// Utility function to sanitize regex input to prevent ReDoS attacks
export const sanitizeRegexInput = securitySanitizeRegex;

// Validation result handler
export const handleValidationErrors = (req: Request, res: Response, next: NextFunction) => {
const errors = validationResult(req);
Expand Down Expand Up @@ -147,4 +151,31 @@ export const validateProjectQuery = [
.custom(isValidProjectId)
.withMessage('Invalid project ID in query'),
handleValidationErrors
];

// Pull Request Query Validation
export const validatePRQuery = [
query('search')
.optional()
.isLength({ max: 100 })
.withMessage('Search query cannot exceed 100 characters')
.matches(/^[a-zA-Z0-9\s\-_.,!?'"()]+$/)
.withMessage('Search query contains invalid characters'),
query('status')
.optional()
.isIn(['open', 'closed', 'merged', 'all'])
.withMessage('Status must be open, closed, merged, or all'),
query('assignedTo')
.optional()
.isIn(['me', 'unassigned'])
.withMessage('AssignedTo must be me or unassigned'),
query('page')
.optional()
.isInt({ min: 1, max: 1000 })
.withMessage('Page must be between 1 and 1000'),
query('limit')
.optional()
.isInt({ min: 1, max: 100 })
.withMessage('Limit must be between 1 and 100'),
handleValidationErrors
];
21 changes: 21 additions & 0 deletions backend/src/routes/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import jwt from "jsonwebtoken";
import authMiddleware, { AuthRequest } from "../middleware/auth";
import { validateLogin, validateSignup } from "../middleware/validation";
import { authLimiter } from "../middleware/security";
import { validateSecureStrings, logSecurityEvent } from "../utils/security";

const router = Router();

Expand All @@ -13,6 +14,16 @@ router.post("/signup", authLimiter, validateSignup, async (req: AuthRequest, res
try {
const { username, email, password } = req.body;

// Ensure all inputs are strings to prevent NoSQL injection
const validation = validateSecureStrings({ email, username, password });
if (!validation.isValid) {
logSecurityEvent('INVALID_INPUT_TYPE', `Signup attempt with invalid input types: ${validation.invalidFields.join(', ')}`, req);
return res.status(400).json({
error: "Invalid request",
message: "All fields must be strings"
});
}

// Check if user exists
const existingUser = await User.findOne({ email });
if (existingUser) {
Expand Down Expand Up @@ -54,6 +65,16 @@ router.post("/login", authLimiter, validateLogin, async (req: AuthRequest, res:
try {
const { email, password } = req.body;

// Ensure email and password are strings to prevent NoSQL injection
const validation = validateSecureStrings({ email, password });
if (!validation.isValid) {
logSecurityEvent('NOSQL_INJECTION_ATTEMPT', `Login attempt with invalid input types: ${validation.invalidFields.join(', ')}`, req);
return res.status(400).json({
error: "Invalid request",
message: "Email and password must be strings"
});
}

// Find user and include password for comparison
const user = await User.findOne({ email }).select('+password');
if (!user) {
Expand Down
23 changes: 13 additions & 10 deletions backend/src/routes/pullRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import authMiddleware, { AuthRequest } from '../middleware/auth';
import {
validateCreatePR,
validatePRReview,
validateObjectId
validateObjectId,
validatePRQuery,
sanitizeRegexInput
} from '../middleware/validation';
import { strictLimiter } from '../middleware/security';
import PullRequest from '../models/PullRequest';
Expand Down Expand Up @@ -64,7 +66,7 @@ router.post('/', strictLimiter, authMiddleware, validateCreatePR, async (req: Au
});

// Get pull requests for a repository/project
router.get('/repository/:projectId', authMiddleware, async (req: AuthRequest, res: Response): Promise<void> => {
router.get('/repository/:projectId', authMiddleware, validatePRQuery, async (req: AuthRequest, res: Response): Promise<void> => {
try {
const { projectId } = req.params;
const { status, search, assignedTo, page = 1, limit = 10, simple } = req.query;
Expand All @@ -83,21 +85,22 @@ router.get('/repository/:projectId', authMiddleware, async (req: AuthRequest, re

const filter: any = { repository: projectId };

// Status filter
if (status && status !== 'all') {
// Status filter - ensure it's a string to prevent NoSQL injection
if (status && typeof status === 'string' && status !== 'all') {
filter.status = status;
}

// Search filter (title and description)
if (search) {
// Search filter (title and description) - sanitized to prevent ReDoS
if (search && typeof search === 'string') {
const sanitizedSearch = sanitizeRegexInput(search);
filter.$or = [
{ title: { $regex: search, $options: 'i' } },
{ description: { $regex: search, $options: 'i' } }
{ title: { $regex: sanitizedSearch, $options: 'i' } },
{ description: { $regex: sanitizedSearch, $options: 'i' } }
];
}

// Assigned to filter
if (assignedTo) {
// Assigned to filter - ensure it's a string to prevent NoSQL injection
if (assignedTo && typeof assignedTo === 'string') {
if (assignedTo === 'me') {
filter.assignedReviewers = req.user.id;
} else if (assignedTo === 'unassigned') {
Expand Down
134 changes: 134 additions & 0 deletions backend/src/utils/security.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/**
* Security utilities for preventing common web application vulnerabilities
*/

import mongoose from 'mongoose';

/**
* Validates that an input is a primitive string (prevents NoSQL injection)
* @param value - The value to validate
* @param fieldName - Name of the field for error messages
* @returns boolean indicating if value is a safe string
*/
export const isSecureString = (value: any, fieldName?: string): value is string => {
if (typeof value !== 'string') {
console.log(`Security: Rejected non-string input for ${fieldName || 'field'}: ${typeof value}`);
return false;
}
return true;
};

/**
* Validates multiple fields are secure strings
* @param fields - Object with field names and values
* @returns Object with validation results
*/
export const validateSecureStrings = (fields: Record<string, any>) => {
const invalidFields: string[] = [];

for (const [fieldName, value] of Object.entries(fields)) {
if (!isSecureString(value, fieldName)) {
invalidFields.push(fieldName);
}
}

return {
isValid: invalidFields.length === 0,
invalidFields
};
};

/**
* Sanitizes regex input to prevent ReDoS attacks
* @param input - The input string to sanitize
* @param maxLength - Maximum allowed length (default: 50)
* @returns Sanitized string safe for regex usage
*/
export const sanitizeRegexInput = (input: string, maxLength: number = 50): string => {
if (!isSecureString(input)) {
return '';
}

// Escape special regex characters and limit length
return input
.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') // Escape regex special chars
.substring(0, maxLength); // Limit length to prevent ReDoS
};

/**
* Validates that a value is a valid MongoDB ObjectId
* @param value - The value to validate
* @returns boolean indicating if value is valid ObjectId
*/
export const isValidObjectId = (value: any): value is string => {
return typeof value === 'string' && mongoose.Types.ObjectId.isValid(value);
};

/**
* Validates that a value is a valid project ID (ObjectId, 'global', or 'default')
* @param value - The value to validate
* @returns boolean indicating if value is valid project ID
*/
export const isValidProjectId = (value: any): value is string => {
return typeof value === 'string' &&
(value === 'global' || value === 'default' || mongoose.Types.ObjectId.isValid(value));
};

/**
* Sanitizes pagination parameters
* @param page - Page number
* @param limit - Items per page
* @returns Sanitized pagination object
*/
export const sanitizePagination = (page: any, limit: any) => {
const pageNum = Math.max(1, Math.min(1000, parseInt(page as string) || 1));
const limitNum = Math.max(1, Math.min(100, parseInt(limit as string) || 10));

return {
page: pageNum,
limit: limitNum,
skip: (pageNum - 1) * limitNum
};
};

/**
* Creates a secure filter object for database queries
* @param baseFilter - Base filter object
* @param allowedFields - Array of allowed field names
* @param inputFilter - User-provided filter object
* @returns Sanitized filter object
*/
export const createSecureFilter = (
baseFilter: Record<string, any>,
allowedFields: string[],
inputFilter: Record<string, any>
): Record<string, any> => {
const secureFilter = { ...baseFilter };

for (const [key, value] of Object.entries(inputFilter)) {
if (allowedFields.includes(key) && isSecureString(value)) {
secureFilter[key] = value;
}
}

return secureFilter;
};

/**
* Logs security events for monitoring
* @param event - Type of security event
* @param details - Additional details about the event
* @param request - Express request object (optional)
*/
export const logSecurityEvent = (
event: string,
details: string,
request?: any
) => {
const timestamp = new Date().toISOString();
const ip = request?.ip || 'unknown';
const userId = request?.user?.id || 'anonymous';
const username = request?.user?.username || 'anonymous';

console.log(`[SECURITY] ${timestamp} - ${event}: ${details} | IP: ${ip} | User: ${username}(${userId})`);
};
Loading