From 47c35c1e3ad097b7006986ed88c46bde8af0ecec Mon Sep 17 00:00:00 2001 From: BizzleBot Date: Tue, 17 Feb 2026 02:09:26 +0000 Subject: [PATCH] security: QA Council audit fixes - Remove JWT secret fallbacks (fail on startup if missing) - Hash refresh tokens with SHA-256 before DB storage - Add Zod validation to bulk-approve, bulk-reject, update-user, copy-week - Add global API rate limiting (100 req/15min per IP) - Fix nginx X-XSS-Protection header (align with Helmet) - Remove --accept-data-loss from Dockerfile CMD - Create .gitignore to protect secrets from commits - Secure .env file permissions (chmod 600) --- .gitignore | 15 +++++++++++++++ backend/src/index.js | 12 ++++++++++++ backend/src/middleware/auth.js | 18 ++++++++++++++++-- backend/src/routes/admin.js | 23 ++++++++--------------- backend/src/routes/auth.js | 13 +++++++------ backend/src/routes/entries.js | 8 +++----- backend/src/utils/validation.js | 20 ++++++++++++++++++++ docker/backend/Dockerfile | 2 +- docker/nginx/default.conf | 2 +- 9 files changed, 83 insertions(+), 30 deletions(-) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..85c7876 --- /dev/null +++ b/.gitignore @@ -0,0 +1,15 @@ +# Secrets +.env +.env.local +.env.*.local +docker/.env +*.pem +*.key + +# Dependencies +node_modules/ + +# Runtime +*.log +.DS_Store +dist/ diff --git a/backend/src/index.js b/backend/src/index.js index f9d5106..c018614 100644 --- a/backend/src/index.js +++ b/backend/src/index.js @@ -1,6 +1,7 @@ const express = require('express'); const cors = require('cors'); const helmet = require('helmet'); +const rateLimit = require('express-rate-limit'); const { PrismaClient } = require('@prisma/client'); const authRoutes = require('./routes/auth'); @@ -38,6 +39,17 @@ app.use( }) ); +// Global rate limiting (100 requests per 15 minutes per IP) +const apiLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + max: 100, + message: { error: 'Too many requests, please try again later.' }, + standardHeaders: true, + legacyHeaders: false, + keyGenerator: (req) => req.ip, +}); +app.use('/api/', apiLimiter); + // Body parsing app.use(express.json({ limit: '1mb' })); diff --git a/backend/src/middleware/auth.js b/backend/src/middleware/auth.js index 21d6aea..6504bad 100644 --- a/backend/src/middleware/auth.js +++ b/backend/src/middleware/auth.js @@ -1,7 +1,20 @@ const jwt = require('jsonwebtoken'); +const crypto = require('crypto'); -const JWT_SECRET = process.env.JWT_SECRET || 'dev-jwt-secret-change-me'; -const JWT_REFRESH_SECRET = process.env.JWT_REFRESH_SECRET || 'dev-refresh-secret-change-me'; +/** + * Hash a refresh token for secure DB storage + */ +function hashRefreshToken(token) { + return crypto.createHash('sha256').update(token).digest('hex'); +} + +const JWT_SECRET = process.env.JWT_SECRET; +const JWT_REFRESH_SECRET = process.env.JWT_REFRESH_SECRET; + +if (!JWT_SECRET || !JWT_REFRESH_SECRET) { + console.error('FATAL: JWT_SECRET and JWT_REFRESH_SECRET environment variables are required'); + process.exit(1); +} const ACCESS_TOKEN_EXPIRY = '15m'; const REFRESH_TOKEN_EXPIRY = '7d'; @@ -93,6 +106,7 @@ module.exports = { generateRefreshToken, verifyAccessToken, verifyRefreshToken, + hashRefreshToken, authenticate, requireAdmin, requireSuperAdmin, diff --git a/backend/src/routes/admin.js b/backend/src/routes/admin.js index d1780de..9fabd48 100644 --- a/backend/src/routes/admin.js +++ b/backend/src/routes/admin.js @@ -3,7 +3,9 @@ const bcrypt = require('bcryptjs'); const { authenticate, requireAdmin } = require('../middleware/auth'); const { approveRejectSchema, + bulkOperationSchema, createUserSchema, + updateUserSchema, createHomeownerSchema, updateHomeownerSchema, reportQuerySchema, @@ -142,15 +144,9 @@ router.get('/timesheets/:id', async (req, res) => { }); // ─────────────── POST /api/admin/timesheets/bulk-approve ─────────────── -router.post('/timesheets/bulk-approve', async (req, res) => { +router.post('/timesheets/bulk-approve', validateBody(bulkOperationSchema), async (req, res) => { try { - const { timesheetIds, notes } = req.body; - if (!Array.isArray(timesheetIds) || timesheetIds.length === 0) { - return res.status(400).json({ error: 'timesheetIds array is required' }); - } - if (timesheetIds.length > 100) { - return res.status(400).json({ error: 'Maximum 100 timesheets per batch' }); - } + const { timesheetIds, notes } = req.validated; const results = { approved: 0, failed: [] }; for (const id of timesheetIds) { @@ -173,12 +169,9 @@ router.post('/timesheets/bulk-approve', async (req, res) => { }); // ─────────────── POST /api/admin/timesheets/bulk-reject ─────────────── -router.post('/timesheets/bulk-reject', async (req, res) => { +router.post('/timesheets/bulk-reject', validateBody(bulkOperationSchema), async (req, res) => { try { - const { timesheetIds, notes } = req.body; - if (!Array.isArray(timesheetIds) || timesheetIds.length === 0) { - return res.status(400).json({ error: 'timesheetIds array is required' }); - } + const { timesheetIds, notes } = req.validated; const results = { rejected: 0, failed: [] }; for (const id of timesheetIds) { @@ -395,10 +388,10 @@ router.post('/users', validateBody(createUserSchema), async (req, res) => { }); // ─────────────── PUT /api/admin/users/:id ─────────────── -router.put('/users/:id', async (req, res) => { +router.put('/users/:id', validateBody(updateUserSchema), async (req, res) => { try { const { id } = req.params; - const { name, role, isActive, password } = req.body; + const { name, role, isActive, password } = req.validated; const user = await req.prisma.user.findUnique({ where: { id } }); if (!user) { diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index 2082489..2c2aac1 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -5,6 +5,7 @@ const { generateAccessToken, generateRefreshToken, verifyRefreshToken, + hashRefreshToken, authenticate, requireAdmin, } = require('../middleware/auth'); @@ -49,10 +50,10 @@ router.post('/login', authLimiter, validateBody(loginSchema), async (req, res) = const accessToken = generateAccessToken(user); const refreshToken = generateRefreshToken(user); - // Store refresh token hash in DB + // Store hashed refresh token in DB await req.prisma.user.update({ where: { id: user.id }, - data: { refreshToken }, + data: { refreshToken: hashRefreshToken(refreshToken) }, }); res.json({ @@ -138,8 +139,8 @@ router.post('/refresh', validateBody(refreshSchema), async (req, res) => { return res.status(401).json({ error: 'User not found or deactivated' }); } - // Verify the refresh token matches the stored one (token rotation) - if (user.refreshToken !== refreshToken) { + // Verify the refresh token hash matches the stored one (token rotation) + if (user.refreshToken !== hashRefreshToken(refreshToken)) { // Possible token theft — invalidate all tokens for this user await req.prisma.user.update({ where: { id: user.id }, @@ -151,10 +152,10 @@ router.post('/refresh', validateBody(refreshSchema), async (req, res) => { const newAccessToken = generateAccessToken(user); const newRefreshToken = generateRefreshToken(user); - // Rotate refresh token + // Rotate refresh token (store hash) await req.prisma.user.update({ where: { id: user.id }, - data: { refreshToken: newRefreshToken }, + data: { refreshToken: hashRefreshToken(newRefreshToken) }, }); res.json({ diff --git a/backend/src/routes/entries.js b/backend/src/routes/entries.js index a347eba..49497be 100644 --- a/backend/src/routes/entries.js +++ b/backend/src/routes/entries.js @@ -4,6 +4,7 @@ const { createEntrySchema, updateEntrySchema, weekQuerySchema, + copyWeekSchema, validateBody, validateQuery, } = require('../utils/validation'); @@ -269,12 +270,9 @@ router.delete('/:id', async (req, res) => { }); // ─────────────── POST /api/entries/copy-week ─────────────── -router.post('/copy-week', async (req, res) => { +router.post('/copy-week', validateBody(copyWeekSchema), async (req, res) => { try { - const { fromWeek, toWeek } = req.body; - if (!fromWeek || !toWeek) { - return res.status(400).json({ error: 'fromWeek and toWeek are required (YYYY-MM-DD Monday)' }); - } + const { fromWeek, toWeek } = req.validated; const fromMonday = getWeekMonday(fromWeek); const toMonday = getWeekMonday(toWeek); diff --git a/backend/src/utils/validation.js b/backend/src/utils/validation.js index 191e9ef..8505334 100644 --- a/backend/src/utils/validation.js +++ b/backend/src/utils/validation.js @@ -69,6 +69,23 @@ const approveRejectSchema = z.object({ notes: z.string().max(1000).trim().optional(), }); +const bulkOperationSchema = z.object({ + timesheetIds: z.array(z.string().uuid('Invalid timesheet ID')).min(1, 'At least one timesheet ID required').max(100, 'Maximum 100 timesheets per batch'), + notes: z.string().max(1000).trim().optional(), +}); + +const updateUserSchema = z.object({ + name: z.string().min(1).max(100).trim().optional(), + role: z.enum(['employee', 'office', 'admin', 'super_admin']).optional(), + isActive: z.boolean().optional(), + password: z.string().min(8, 'Password must be at least 8 characters').max(128).optional(), +}); + +const copyWeekSchema = z.object({ + fromWeek: z.string().regex(/^\d{4}-\d{2}-\d{2}$/, 'fromWeek must be YYYY-MM-DD'), + toWeek: z.string().regex(/^\d{4}-\d{2}-\d{2}$/, 'toWeek must be YYYY-MM-DD'), +}); + const createUserSchema = registerSchema; const createHomeownerSchema = z.object({ @@ -138,6 +155,9 @@ module.exports = { submitTimesheetSchema, emailTimesheetSchema, approveRejectSchema, + bulkOperationSchema, + updateUserSchema, + copyWeekSchema, createUserSchema, createHomeownerSchema, updateHomeownerSchema, diff --git a/docker/backend/Dockerfile b/docker/backend/Dockerfile index 40c12fb..9cbb99d 100644 --- a/docker/backend/Dockerfile +++ b/docker/backend/Dockerfile @@ -55,4 +55,4 @@ HEALTHCHECK --interval=30s --timeout=5s --start-period=30s --retries=3 \ CMD node -e "fetch('http://localhost:3001/api/health').then(r=>{if(!r.ok)throw 1}).catch(()=>process.exit(1))" # Start with migration and seed on first run -CMD ["sh", "-c", "npx prisma db push --accept-data-loss 2>/dev/null; node prisma/seed.js 2>/dev/null; node src/index.js"] +CMD ["sh", "-c", "npx prisma db push 2>/dev/null; node prisma/seed.js 2>/dev/null; node src/index.js"] diff --git a/docker/nginx/default.conf b/docker/nginx/default.conf index aac9bc1..cde0814 100644 --- a/docker/nginx/default.conf +++ b/docker/nginx/default.conf @@ -15,7 +15,7 @@ server { # Security headers add_header X-Frame-Options "SAMEORIGIN" always; add_header X-Content-Type-Options "nosniff" always; - add_header X-XSS-Protection "1; mode=block" always; + add_header X-XSS-Protection "0" always; add_header Referrer-Policy "strict-origin-when-cross-origin" always; # Gzip compression