fix(#242): harden access control and atomicity from code review

server/src/routes/namedResources.ts

@@ -45,10 +45,6 @@ router.post('/', asyncHandler(async (req: AuthRequest, res: Response) => {
   const rt = await verifyResourceType(rtId, projectId)
   if (!rt) { res.status(404).json({ error: 'Resource type not found' }); return }
 
-  if (rt.allocationMode === 'CAPACITY_PLAN') {
-    await exitCapacityPlanForManualScheduling(rt.id)
-  }
-
   const { name: rawName, startWeek, endWeek, allocationPct, pricingModel } = req.body
 
   // Auto-generate a numbered name if none provided or generic
@@ -75,27 +71,35 @@ router.post('/', asyncHandler(async (req: AuthRequest, res: Response) => {
   const rtAllocationEndWeek = rt.allocationMode === 'CAPACITY_PLAN' ? null : (rt.allocationEndWeek ?? null)
   const inheritAllocation = rtAllocationMode !== 'EFFORT'
 
-  const resource = await prisma.namedResource.create({
-    data: {
-      name,
-      resourceTypeId: rtId,
-      ...(startWeek !== undefined && { startWeek }),
-      ...(endWeek !== undefined && { endWeek }),
-      ...(allocationPct !== undefined && { allocationPct }),
-      ...(pricingModel !== undefined && { pricingModel }),
-      ...(inheritAllocation && {
-        allocationMode: rtAllocationMode,
-        allocationPercent: rtAllocationPercent,
-        allocationStartWeek: rtAllocationStartWeek,
-        allocationEndWeek: rtAllocationEndWeek,
-      }),
-    },
+  const resource = await prisma.$transaction(async tx => {
+    if (rt.allocationMode === 'CAPACITY_PLAN') {
+      await exitCapacityPlanForManualScheduling(rt.id, tx)
+    }
+
+    const created = await tx.namedResource.create({
+      data: {
+        name,
+        resourceTypeId: rtId,
+        ...(startWeek !== undefined && { startWeek }),
+        ...(endWeek !== undefined && { endWeek }),
+        ...(allocationPct !== undefined && { allocationPct }),
+        ...(pricingModel !== undefined && { pricingModel }),
+        ...(inheritAllocation && {
+          allocationMode: rtAllocationMode,
+          allocationPercent: rtAllocationPercent,
+          allocationStartWeek: rtAllocationStartWeek,
+          allocationEndWeek: rtAllocationEndWeek,
+        }),
+      },
+    })
+
+    // Sync resource type count to match total named resources
+    const total = await tx.namedResource.count({ where: { resourceTypeId: rtId } })
+    await tx.resourceType.update({ where: { id: rtId }, data: { count: total } })
+
+    return created
   })
 
-  // Sync resource type count to match total named resources
-  const total = await prisma.namedResource.count({ where: { resourceTypeId: rtId } })
-  await prisma.resourceType.update({ where: { id: rtId }, data: { count: total } })
-
   res.status(201).json(resource)
 }))
 
@@ -173,15 +177,17 @@ router.delete('/:id', asyncHandler(async (req: AuthRequest, res: Response) => {
   const existing = await prisma.namedResource.findFirst({ where: { id, resourceTypeId: rtId } })
   if (!existing) { res.status(404).json({ error: 'Named resource not found' }); return }
 
-  if (rt.allocationMode === 'CAPACITY_PLAN') {
-    await exitCapacityPlanForManualScheduling(rt.id)
-  }
+  await prisma.$transaction(async tx => {
+    if (rt.allocationMode === 'CAPACITY_PLAN') {
+      await exitCapacityPlanForManualScheduling(rt.id, tx)
+    }
 
-  await prisma.namedResource.delete({ where: { id } })
+    await tx.namedResource.delete({ where: { id } })
 
-  // Sync resource type count (can reach 0 when all named resources are deleted)
-  const total = await prisma.namedResource.count({ where: { resourceTypeId: rtId } })
-  await prisma.resourceType.update({ where: { id: rtId }, data: { count: total } })
+    // Sync resource type count (can reach 0 when all named resources are deleted)
+    const total = await tx.namedResource.count({ where: { resourceTypeId: rtId } })
+    await tx.resourceType.update({ where: { id: rtId }, data: { count: total } })
+  })
 
   res.status(204).send()
 }))

server/src/routes/resourceTypes.ts

@@ -209,7 +209,8 @@ router.patch('/:id', asyncHandler(async (req: AuthRequest, res: Response) => {
 router.delete('/:id', asyncHandler(async (req: AuthRequest, res: Response) => {
   const project = await ownedProject(req.params.projectId as string, req.userId!)
   if (!project) { res.status(404).json({ error: 'Project not found' }); return }
-  await prisma.resourceType.delete({ where: { id: req.params.id as string } })
+  const result = await prisma.resourceType.deleteMany({ where: { id: req.params.id as string, projectId: req.params.projectId as string } })
+  if (result.count === 0) { res.status(404).json({ error: 'Resource type not found' }); return }
   res.json({ message: 'Deleted' })
 }))
 

server/src/routes/timeline.ts

@@ -960,6 +960,9 @@ router.put('/:featureId', asyncHandler(async (req: AuthRequest, res: Response) =
   }
 
   const featureId = req.params.featureId as string
+  const feature = await prisma.feature.findFirst({ where: { id: featureId, epic: { projectId: project.id } } })
+  if (!feature) { res.status(404).json({ error: 'Feature not found' }); return }
+
   const entry = await prisma.timelineEntry.upsert({
     where: { featureId },
     create: { projectId: project.id, featureId, startWeek, durationWeeks, isManual: true },

server/src/test/resourceTypes.test.ts

@@ -294,6 +294,8 @@ describe('resource type manual scheduling regression', () => {
       },
       namedResource: {
         updateMany: vi.fn().mockResolvedValue({ count: 2 }),
+        delete: vi.fn().mockResolvedValue({}),
+        count: vi.fn().mockResolvedValue(1),
       },
     }
     vi.mocked(prisma.$transaction).mockImplementation(async (fn: any) => fn(exitTx))
@@ -331,9 +333,45 @@ describe('resource type manual scheduling regression', () => {
         endWeek: null,
       },
     })
-    expect(prisma.resourceType.update).toHaveBeenCalledWith({
+    expect(exitTx.namedResource.delete).toHaveBeenCalledWith({ where: { id: 'nr-2' } })
+    expect(exitTx.resourceType.update).toHaveBeenCalledWith({
       where: { id: 'rt-1' },
       data: { count: 1 },
     })
   })
 })
+
+describe('DELETE /api/projects/:projectId/resource-types/:id', () => {
+  it('deletes a resource type scoped to the project', async () => {
+    vi.mocked(prisma.project.findFirst).mockResolvedValue({ id: 'proj-1', ownerId: userId } as never)
+    vi.mocked(prisma.resourceType.deleteMany).mockResolvedValue({ count: 1 } as never)
+
+    const res = await request(app)
+      .delete('/api/projects/proj-1/resource-types/rt-1')
+      .set('Authorization', authHeader)
+
+    expect(res.status).toBe(200)
+    expect(res.body).toEqual({ message: 'Deleted' })
+    // Delete must be scoped to the project, not a bare primary-key delete
+    expect(prisma.resourceType.deleteMany).toHaveBeenCalledWith({
+      where: { id: 'rt-1', projectId: 'proj-1' },
+    })
+  })
+
+  it('returns 404 when the resource type belongs to another project (cross-tenant delete)', async () => {
+    // Caller owns proj-1, but rt-99 lives in another tenant's project, so the
+    // project-scoped deleteMany affects 0 rows.
+    vi.mocked(prisma.project.findFirst).mockResolvedValue({ id: 'proj-1', ownerId: userId } as never)
+    vi.mocked(prisma.resourceType.deleteMany).mockResolvedValue({ count: 0 } as never)
+
+    const res = await request(app)
+      .delete('/api/projects/proj-1/resource-types/rt-99')
+      .set('Authorization', authHeader)
+
+    expect(res.status).toBe(404)
+    expect(res.body).toEqual({ error: 'Resource type not found' })
+    expect(prisma.resourceType.deleteMany).toHaveBeenCalledWith({
+      where: { id: 'rt-99', projectId: 'proj-1' },
+    })
+  })
+})

server/src/test/setup.ts

@@ -19,7 +19,7 @@ vi.mock('../lib/prisma.js', () => ({
     feature: { findMany: vi.fn(), findFirst: vi.fn(), create: vi.fn(), update: vi.fn(), delete: vi.fn(), count: vi.fn() },
     userStory: { findMany: vi.fn(), findFirst: vi.fn(), findUnique: vi.fn(), create: vi.fn(), update: vi.fn(), delete: vi.fn(), count: vi.fn() },
     task: { findMany: vi.fn(), findFirst: vi.fn(), create: vi.fn(), update: vi.fn(), delete: vi.fn(), count: vi.fn() },
-    resourceType: { findMany: vi.fn(), findFirst: vi.fn(), create: vi.fn(), createMany: vi.fn(), update: vi.fn(), updateMany: vi.fn(), delete: vi.fn() },
+    resourceType: { findMany: vi.fn(), findFirst: vi.fn(), create: vi.fn(), createMany: vi.fn(), update: vi.fn(), updateMany: vi.fn(), delete: vi.fn(), deleteMany: vi.fn() },
     globalResourceType: { findMany: vi.fn(), findFirst: vi.fn(), create: vi.fn(), update: vi.fn(), delete: vi.fn() },
     featureTemplate: { findMany: vi.fn(), findFirst: vi.fn(), findUnique: vi.fn(), create: vi.fn(), update: vi.fn(), delete: vi.fn() },
     templateTask: { findMany: vi.fn(), findFirst: vi.fn(), findUnique: vi.fn(), create: vi.fn(), update: vi.fn(), delete: vi.fn(), deleteMany: vi.fn(), count: vi.fn() },
@@ -59,7 +59,7 @@ vi.mock('../lib/prisma.js', () => ({
       task: { create: vi.fn() },
       project: { update: vi.fn() },
       resourceType: { findUnique: vi.fn().mockResolvedValue(null), findMany: vi.fn(), update: vi.fn(), create: vi.fn(), upsert: vi.fn() },
-      namedResource: { findUnique: vi.fn().mockResolvedValue(null), findMany: vi.fn(), update: vi.fn(), updateMany: vi.fn(), create: vi.fn(), upsert: vi.fn(), delete: vi.fn() },
+      namedResource: { findUnique: vi.fn().mockResolvedValue(null), findMany: vi.fn(), update: vi.fn(), updateMany: vi.fn(), create: vi.fn(), upsert: vi.fn(), delete: vi.fn(), count: vi.fn() },
       timelineEntry: { deleteMany: vi.fn(), createMany: vi.fn() },
       storyTimelineEntry: { deleteMany: vi.fn(), createMany: vi.fn() },
       epicDependency: { deleteMany: vi.fn(), createMany: vi.fn() },

server/src/test/timeline.test.ts

@@ -877,6 +877,7 @@ describe('PUT /api/projects/:projectId/timeline/:featureId', () => {
 
   it('overrides a feature startWeek and durationWeeks with isManual=true', async () => {
     vi.mocked(prisma.project.findFirst).mockResolvedValue(mockProject as any)
+    vi.mocked(prisma.feature.findFirst).mockResolvedValue({ id: 'feat-1', epicId: 'epic-1' } as any)
     vi.mocked(prisma.timelineEntry.upsert).mockResolvedValue({
       id: 'entry-1',
       projectId: 'proj-1',
@@ -898,6 +899,20 @@ describe('PUT /api/projects/:projectId/timeline/:featureId', () => {
     expect(res.body.isManual).toBe(true)
   })
 
+  it('returns 404 when the feature belongs to another project (cross-tenant write)', async () => {
+    vi.mocked(prisma.project.findFirst).mockResolvedValue(mockProject as any)
+    // Feature ownership scoped to the authorised project — foreign feature not found
+    vi.mocked(prisma.feature.findFirst).mockResolvedValue(null)
+
+    const res = await request(app)
+      .put('/api/projects/proj-1/timeline/feat-foreign')
+      .set('Authorization', authHeader)
+      .send({ startWeek: 2, durationWeeks: 3 })
+
+    expect(res.status).toBe(404)
+    expect(prisma.timelineEntry.upsert).not.toHaveBeenCalled()
+  })
+
   it('returns 400 when startWeek or durationWeeks missing', async () => {
     vi.mocked(prisma.project.findFirst).mockResolvedValue(mockProject as any)
     const res = await request(app)