From 9d0beed02f7333b27623b3efb00447c14ef9c3ab Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 29 Apr 2026 03:30:00 +0200 Subject: [PATCH] fix(security): file storage authorization hardening Three separate issues in the file storage layer: 1. IDOR via client-controlled object key in applicant.saveFileMetadata and file.replaceFile. Both procedures accepted `bucket` and `objectKey` from the client and stored them on a new ProjectFile row attached to the caller's own project. Because file.getDownloadUrl authorizes via `findFirst({ bucket, objectKey })` -> projectId, an attacker could bind another team's storage object to their own project row and then download the foreign object through the legitimate authorization path. Now both procedures require `bucket === BUCKET_NAME` and the `objectKey` to start with the project's sanitized title prefix (matches the prefix that generateObjectKey produces server-side). New helper `objectKeyBelongsToProject` exported from src/lib/minio.ts; `sanitizePath` is now exported as well so the helper can reuse it. 2. Missing per-round scope on file.getBulkDownloadUrls. The single-file getDownloadUrl restricts a juror to files in rounds with sortOrder <= their assigned round, but the bulk variant only checked that an Assignment row existed for the project. A juror assigned only to EVALUATION could pull URLs for LIVE_FINAL/DELIBERATION confidential files via this endpoint. Now applies the same per-round filter when the caller's access to the project is jury-only (mentors / team members / award jurors retain unrestricted access, matching getDownloadUrl semantics). 3. Same omission on the standalone /api/files/bulk-download REST route. Same fix applied there. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/api/files/bulk-download/route.ts | 55 ++++++++++++++------ src/lib/minio.ts | 15 +++++- src/server/routers/applicant.ts | 20 +++++++- src/server/routers/file.ts | 64 +++++++++++++++++++++++- 4 files changed, 136 insertions(+), 18 deletions(-) diff --git a/src/app/api/files/bulk-download/route.ts b/src/app/api/files/bulk-download/route.ts index 065b237..f398599 100644 --- a/src/app/api/files/bulk-download/route.ts +++ b/src/app/api/files/bulk-download/route.ts @@ -30,21 +30,21 @@ export async function POST(request: NextRequest): Promise { // Authorization: must be admin or assigned jury/mentor for this project const isAdmin = userRole === 'SUPER_ADMIN' || userRole === 'PROGRAM_ADMIN' + // Per-round scope: jurors may only pull URLs for files in rounds with + // sortOrder <= their assigned round. Mirrors file.getDownloadUrl. Without + // this, a juror assigned to EVALUATION could bulk-download LIVE_FINAL + // confidential files via this endpoint. + let priorRoundIds: string[] | null = null + if (!isAdmin) { - // Check if user is assigned as jury const juryAssignment = await prisma.assignment.findFirst({ - where: { - userId, - projectId, - }, + where: { userId, projectId }, + select: { id: true, roundId: true }, }) - // Check if user is assigned as mentor const mentorAssignment = await prisma.mentorAssignment.findFirst({ - where: { - mentorId: userId, - projectId, - }, + where: { mentorId: userId, projectId }, + select: { id: true }, }) if (!juryAssignment && !mentorAssignment) { @@ -53,14 +53,41 @@ export async function POST(request: NextRequest): Promise { { status: 403 } ) } + + // Apply the per-round filter only when access is jury-only. + if (juryAssignment && !mentorAssignment) { + const assignedRound = await prisma.round.findUnique({ + where: { id: juryAssignment.roundId }, + select: { competitionId: true, sortOrder: true }, + }) + if (assignedRound) { + const priorOrCurrent = await prisma.round.findMany({ + where: { + competitionId: assignedRound.competitionId, + sortOrder: { lte: assignedRound.sortOrder }, + }, + select: { id: true }, + }) + priorRoundIds = priorOrCurrent.map((r) => r.id) + } + } } // Fetch file metadata from DB + const fileWhere: Record = { + id: { in: fileIds }, + projectId, + } + if (priorRoundIds !== null) { + fileWhere.OR = [ + { requirement: { roundId: { in: priorRoundIds } } }, + { requirementId: null, roundId: { in: priorRoundIds } }, + { requirementId: null, roundId: null }, + ] + } + const files = await prisma.projectFile.findMany({ - where: { - id: { in: fileIds }, - projectId, - }, + where: fileWhere, select: { id: true, fileName: true, diff --git a/src/lib/minio.ts b/src/lib/minio.ts index fd4bd36..5ab9a17 100644 --- a/src/lib/minio.ts +++ b/src/lib/minio.ts @@ -116,7 +116,7 @@ export async function deleteObject( * Sanitize a name for use as a MinIO path segment. * Removes special characters, replaces spaces with underscores, limits length. */ -function sanitizePath(name: string): string { +export function sanitizePath(name: string): string { return ( name .trim() @@ -164,3 +164,16 @@ export function generateMentorObjectKey( return generateObjectKey(projectTitle, fileName, 'mentorship') } +/** + * Verify a client-supplied object key actually belongs to a project's + * sanitized prefix. Used to prevent cross-tenant binding where an attacker + * passes another team's `bucket+objectKey` into a metadata-save procedure. + */ +export function objectKeyBelongsToProject( + objectKey: string, + projectTitle: string, +): boolean { + const sanitized = sanitizePath(projectTitle) + return objectKey.startsWith(`${sanitized}/`) +} + diff --git a/src/server/routers/applicant.ts b/src/server/routers/applicant.ts index d0f464c..a4a615c 100644 --- a/src/server/routers/applicant.ts +++ b/src/server/routers/applicant.ts @@ -2,7 +2,7 @@ import crypto from 'crypto' import { z } from 'zod' import { TRPCError } from '@trpc/server' import { router, publicProcedure, protectedProcedure } from '../trpc' -import { getPresignedUrl, generateObjectKey, BUCKET_NAME } from '@/lib/minio' +import { getPresignedUrl, generateObjectKey, BUCKET_NAME, objectKeyBelongsToProject } from '@/lib/minio' import { generateLogoKey, createStorageProvider, type StorageProviderType } from '@/lib/storage' import { getImageUploadUrl, confirmImageUpload, getImageUrl, deleteImage, type ImageUploadConfig } from '@/server/utils/image-upload' import { sendStyledNotificationEmail, sendTeamMemberInviteEmail } from '@/lib/email' @@ -408,6 +408,24 @@ export const applicantRouter = router({ }) } + // Verify the client-supplied objectKey actually belongs to this project's + // sanitized prefix. Without this, a user could pass another team's + // bucket+objectKey here, attach it to their own project row, and then + // download the foreign object via file.getDownloadUrl (which authorizes + // by bucket+objectKey -> projectId). + if (input.bucket !== BUCKET_NAME) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: 'Invalid file location', + }) + } + if (!objectKeyBelongsToProject(input.objectKey, project.title)) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: 'Invalid file location for this project', + }) + } + // Block rejected projects if (await isProjectRejected(ctx.prisma, input.projectId)) { throw new TRPCError({ code: 'FORBIDDEN', message: 'Your project has been rejected. File changes are no longer permitted.' }) diff --git a/src/server/routers/file.ts b/src/server/routers/file.ts index 2075873..ca43edb 100644 --- a/src/server/routers/file.ts +++ b/src/server/routers/file.ts @@ -1,7 +1,7 @@ import { z } from 'zod' import { TRPCError } from '@trpc/server' import { router, protectedProcedure, adminProcedure } from '../trpc' -import { getPresignedUrl, generateObjectKey, deleteObject, BUCKET_NAME } from '@/lib/minio' +import { getPresignedUrl, generateObjectKey, deleteObject, BUCKET_NAME, objectKeyBelongsToProject } from '@/lib/minio' import { logAudit } from '../utils/audit' import { checkRequirementsAndTransition } from '../services/round-engine' @@ -556,6 +556,28 @@ export const fileRouter = router({ }) } + // Verify the client-supplied objectKey actually belongs to this project's + // sanitized prefix. Without this check, a team member (or admin) could + // pass another project's bucket+objectKey here and bind it as a "new + // version" — then anyone with project access could read the foreign + // object via file.getDownloadUrl, which authorizes by bucket+objectKey. + const project = await ctx.prisma.project.findUniqueOrThrow({ + where: { id: input.projectId }, + select: { title: true }, + }) + if (input.bucket !== BUCKET_NAME) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: 'Invalid file location', + }) + } + if (!objectKeyBelongsToProject(input.objectKey, project.title)) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: 'Invalid file location for this project', + }) + } + // Create new file and update old file in a transaction const result = await ctx.prisma.$transaction(async (tx) => { const newFile = await tx.projectFile.create({ @@ -696,11 +718,18 @@ export const fileRouter = router({ .query(async ({ ctx, input }) => { const isAdminOrObserver = ['SUPER_ADMIN', 'PROGRAM_ADMIN', 'OBSERVER', 'AWARD_MASTER'].includes(ctx.user.role) + // For non-admin/observer callers, mirror the per-round scope used by + // file.getDownloadUrl: a juror assigned to round N may only pull URLs + // for files in rounds with sortOrder <= N. Without this, a juror + // assigned to EVALUATION can bulk-download LIVE_FINAL/DELIBERATION + // confidential files via this endpoint. + let priorRoundIds: string[] | null = null + if (!isAdminOrObserver) { const [assignment, mentorAssignment, teamMembership] = await Promise.all([ ctx.prisma.assignment.findFirst({ where: { userId: ctx.user.id, projectId: input.projectId }, - select: { id: true }, + select: { id: true, roundId: true }, }), ctx.prisma.mentorAssignment.findFirst({ where: { mentorId: ctx.user.id, projectId: input.projectId }, @@ -724,6 +753,26 @@ export const fileRouter = router({ message: 'You do not have access to this project\'s files', }) } + + // Apply per-round filter only when the caller's access is jury-only. + // Mentors / team members / award jurors get unrestricted file access + // on the project (matches getDownloadUrl semantics). + if (assignment && !mentorAssignment && !teamMembership) { + const assignedRound = await ctx.prisma.round.findUnique({ + where: { id: assignment.roundId }, + select: { competitionId: true, sortOrder: true }, + }) + if (assignedRound) { + const priorOrCurrent = await ctx.prisma.round.findMany({ + where: { + competitionId: assignedRound.competitionId, + sortOrder: { lte: assignedRound.sortOrder }, + }, + select: { id: true }, + }) + priorRoundIds = priorOrCurrent.map((r) => r.id) + } + } } // Get files @@ -731,6 +780,17 @@ export const fileRouter = router({ if (input.fileIds && input.fileIds.length > 0) { where.id = { in: input.fileIds } } + if (priorRoundIds !== null) { + // Allow a file when: + // - it has a requirement whose round is in the allowed set, OR + // - it has no requirement and its roundId is in the allowed set, OR + // - it has neither requirement nor roundId (legacy general files) + where.OR = [ + { requirement: { roundId: { in: priorRoundIds } } }, + { requirementId: null, roundId: { in: priorRoundIds } }, + { requirementId: null, roundId: null }, + ] + } const files = await ctx.prisma.projectFile.findMany({ where,