Comprehensive platform review: security fixes, query optimization, UI improvements, and code cleanup
Security (Critical/High): - Fix path traversal bypass in local storage provider (path.resolve + prefix check) - Fix timing-unsafe HMAC comparison (crypto.timingSafeEqual) - Add auth + ownership checks to email API routes (verify-credentials, change-password) - Remove hardcoded secret key fallback in local storage provider - Add production credential check for MinIO (fail loudly if not set) - Remove DB error details from health check response - Add stricter rate limiting on application submissions (5/hour) - Add rate limiting on email availability check (anti-enumeration) - Change getAIAssignmentJobStatus to adminProcedure - Block dangerous file extensions on upload - Reduce project list max perPage from 5000 to 200 Query Optimization: - Optimize analytics getProjectRankings with select instead of full includes - Fix N+1 in mentor.getSuggestions (batch findMany instead of loop) - Use _count for files instead of fetching full file records in project list - Switch to bulk notifications in assignment and user bulk operations - Batch filtering upserts (25 per transaction instead of all at once) UI/UX: - Replace Inter font with Montserrat in public layout (brand consistency) - Use Logo component in public layout instead of placeholder - Create branded 404 and error pages - Make admin rounds table responsive with mobile card layout - Fix notification bell paths to be role-aware - Replace hardcoded slate colors with semantic tokens in admin sidebar - Force light mode (dark mode untested) - Adjust CardTitle default size - Improve muted-foreground contrast for accessibility (A11Y) - Move profile form state initialization to useEffect Code Quality: - Extract shared toProjectWithRelations to anonymization.ts (removed 3 duplicates) - Remove dead code: getObjectInfo, isValidImageSize, unused batch tag functions, debug logs - Remove unused twilio dependency - Remove redundant email index from schema - Add actual storage object deletion when file records are deleted - Wrap evaluation submit + assignment update in - Add comprehensive platform review document Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -15,12 +15,18 @@ export const MINIO_PUBLIC_ENDPOINT = process.env.MINIO_PUBLIC_ENDPOINT || MINIO_
|
||||
function createMinioClient(): Minio.Client {
|
||||
const url = new URL(MINIO_ENDPOINT)
|
||||
|
||||
const accessKey = process.env.MINIO_ACCESS_KEY
|
||||
const secretKey = process.env.MINIO_SECRET_KEY
|
||||
if (process.env.NODE_ENV === 'production' && (!accessKey || !secretKey)) {
|
||||
throw new Error('MINIO_ACCESS_KEY and MINIO_SECRET_KEY environment variables are required in production')
|
||||
}
|
||||
|
||||
return new Minio.Client({
|
||||
endPoint: url.hostname,
|
||||
port: url.port ? parseInt(url.port) : (url.protocol === 'https:' ? 443 : 80),
|
||||
useSSL: url.protocol === 'https:',
|
||||
accessKey: process.env.MINIO_ACCESS_KEY || 'minioadmin',
|
||||
secretKey: process.env.MINIO_SECRET_KEY || 'minioadmin',
|
||||
accessKey: accessKey || 'minioadmin',
|
||||
secretKey: secretKey || 'minioadmin',
|
||||
})
|
||||
}
|
||||
|
||||
@@ -109,12 +115,3 @@ export function generateObjectKey(
|
||||
return `projects/${projectId}/${timestamp}-${sanitizedName}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Get file metadata from MinIO
|
||||
*/
|
||||
export async function getObjectInfo(
|
||||
bucket: string,
|
||||
objectKey: string
|
||||
): Promise<Minio.BucketItemStat> {
|
||||
return minio.statObject(bucket, objectKey)
|
||||
}
|
||||
|
||||
@@ -129,10 +129,3 @@ export function isValidImageType(contentType: string): boolean {
|
||||
const validTypes = ['image/jpeg', 'image/png', 'image/gif', 'image/webp']
|
||||
return validTypes.includes(contentType)
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate image file size (default 5MB max)
|
||||
*/
|
||||
export function isValidImageSize(sizeBytes: number, maxMB: number = 5): boolean {
|
||||
return sizeBytes <= maxMB * 1024 * 1024
|
||||
}
|
||||
|
||||
@@ -1,9 +1,15 @@
|
||||
import { createHmac } from 'crypto'
|
||||
import { createHmac, timingSafeEqual } from 'crypto'
|
||||
import * as fs from 'fs/promises'
|
||||
import * as path from 'path'
|
||||
import type { StorageProvider } from './types'
|
||||
|
||||
const SECRET_KEY = process.env.NEXTAUTH_SECRET || 'local-storage-secret'
|
||||
function getSecretKey(): string {
|
||||
const key = process.env.NEXTAUTH_SECRET
|
||||
if (!key) {
|
||||
throw new Error('NEXTAUTH_SECRET environment variable is required for local storage signing')
|
||||
}
|
||||
return key
|
||||
}
|
||||
const DEFAULT_BASE_PATH = './uploads'
|
||||
|
||||
/**
|
||||
@@ -31,7 +37,7 @@ export class LocalStorageProvider implements StorageProvider {
|
||||
): string {
|
||||
const expiresAt = Math.floor(Date.now() / 1000) + expirySeconds
|
||||
const payload = `${action}:${key}:${expiresAt}`
|
||||
const signature = createHmac('sha256', SECRET_KEY)
|
||||
const signature = createHmac('sha256', getSecretKey())
|
||||
.update(payload)
|
||||
.digest('hex')
|
||||
|
||||
@@ -55,17 +61,29 @@ export class LocalStorageProvider implements StorageProvider {
|
||||
signature: string
|
||||
): boolean {
|
||||
const payload = `${action}:${key}:${expiresAt}`
|
||||
const expectedSignature = createHmac('sha256', SECRET_KEY)
|
||||
const expectedSignature = createHmac('sha256', getSecretKey())
|
||||
.update(payload)
|
||||
.digest('hex')
|
||||
|
||||
return signature === expectedSignature && expiresAt > Date.now() / 1000
|
||||
// Use timing-safe comparison to prevent timing attacks
|
||||
const sigBuffer = Buffer.from(signature, 'hex')
|
||||
const expectedBuffer = Buffer.from(expectedSignature, 'hex')
|
||||
if (sigBuffer.length !== expectedBuffer.length) {
|
||||
return false
|
||||
}
|
||||
|
||||
return timingSafeEqual(sigBuffer, expectedBuffer) && expiresAt > Date.now() / 1000
|
||||
}
|
||||
|
||||
private getFilePath(key: string): string {
|
||||
// Sanitize key to prevent path traversal
|
||||
const sanitizedKey = key.replace(/\.\./g, '').replace(/^\//, '')
|
||||
return path.join(this.basePath, sanitizedKey)
|
||||
const resolved = path.resolve(this.basePath, sanitizedKey)
|
||||
const resolvedBase = path.resolve(this.basePath)
|
||||
if (!resolved.startsWith(resolvedBase + path.sep) && resolved !== resolvedBase) {
|
||||
throw new Error('Invalid file path: path traversal detected')
|
||||
}
|
||||
return resolved
|
||||
}
|
||||
|
||||
private async ensureDirectory(filePath: string): Promise<void> {
|
||||
|
||||
Reference in New Issue
Block a user