From 8a90395c14dba528297c04bdcddaa530c7625351 Mon Sep 17 00:00:00 2001 From: Dennis Lysenko Date: Mon, 21 Sep 2026 15:11:18 -0400 Subject: [PATCH 1/6] fix(api): derive soa organizationId from session, not request body Every SOA handler now overwrites dto.organizationId with the trusted @OrganizationId() session value before calling the service, closing the cross-tenant read/tamper/destroy gap in save-answer, auto-fill, create-document, ensure-setup, approve, decline, and submit-for-approval. Refs GH-036 --- apps/api/src/soa/soa.controller.spec.ts | 161 +++++++++++++++++++++++- apps/api/src/soa/soa.controller.ts | 7 ++ 2 files changed, 164 insertions(+), 4 deletions(-) diff --git a/apps/api/src/soa/soa.controller.spec.ts b/apps/api/src/soa/soa.controller.spec.ts index 41712e3e88..98fdabee0f 100644 --- a/apps/api/src/soa/soa.controller.spec.ts +++ b/apps/api/src/soa/soa.controller.spec.ts @@ -29,6 +29,8 @@ jest.mock('@/vector-store/lib', () => ({ syncOrganizationEmbeddings: jest.fn(), })); +import { syncOrganizationEmbeddings } from '@/vector-store/lib'; + describe('SOAController', () => { let controller: SOAController; let soaService: jest.Mocked; @@ -98,7 +100,7 @@ describe('SOAController', () => { mockSOAService.saveAnswer.mockResolvedValue({ success: true }); const result = await controller.saveAnswer( - dto as never, + dto, 'org_123', mockAuthContext, ); @@ -112,6 +114,64 @@ describe('SOAController', () => { controller.saveAnswer(dto as never, 'org_123', noUserAuthContext), ).rejects.toThrow(BadRequestException); }); + + it('should overwrite a body-supplied organizationId with the session organizationId', async () => { + const spoofedDto = { ...dto, organizationId: 'org_victim' }; + mockSOAService.saveAnswer.mockResolvedValue({ success: true }); + + await controller.saveAnswer(spoofedDto, 'org_own', mockAuthContext); + + expect(soaService.saveAnswer).toHaveBeenCalledWith( + expect.objectContaining({ organizationId: 'org_own' }), + 'usr_123', + ); + }); + }); + + describe('autoFill', () => { + const dto = { + organizationId: 'org_123', + documentId: 'doc_1', + }; + + it('should overwrite a body-supplied organizationId with the session organizationId', async () => { + const spoofedDto = { ...dto, organizationId: 'org_victim' }; + mockSOAService.getDocument.mockResolvedValue({ + configuration: { questions: [] }, + }); + mockSOAService.checkIfFullyRemote.mockResolvedValue(false); + mockSOAService.batchSearchSOAQuestions.mockResolvedValue(new Map()); + mockSOAService.saveAnswersToDatabase.mockResolvedValue(undefined); + mockSOAService.countAnsweredAnswers.mockResolvedValue(0); + mockSOAService.updateDocumentAfterAutoFill.mockResolvedValue(undefined); + const res = { + setHeader: jest.fn(), + write: jest.fn(), + end: jest.fn(), + } as unknown as Response; + + await controller.autoFill(spoofedDto, 'org_own', mockAuthContext, res); + + expect(syncOrganizationEmbeddings).toHaveBeenCalledWith('org_own'); + expect(soaService.getDocument).toHaveBeenCalledWith('doc_1', 'org_own'); + expect(soaService.checkIfFullyRemote).toHaveBeenCalledWith('org_own'); + expect(soaService.batchSearchSOAQuestions).toHaveBeenCalledWith( + [], + 'org_own', + ); + }); + + it('should throw BadRequestException when userId is missing', async () => { + const res = { + setHeader: jest.fn(), + write: jest.fn(), + end: jest.fn(), + } as unknown as Response; + + await expect( + controller.autoFill(dto as never, 'org_123', noUserAuthContext, res), + ).rejects.toThrow(BadRequestException); + }); }); describe('createDocument', () => { @@ -129,6 +189,17 @@ describe('SOAController', () => { expect(soaService.createDocument).toHaveBeenCalledWith(dto); expect(result).toEqual(created); }); + + it('should overwrite a body-supplied organizationId with the session organizationId', async () => { + const spoofedDto = { ...dto, organizationId: 'org_victim' }; + mockSOAService.createDocument.mockResolvedValue({ id: 'doc_1' }); + + await controller.createDocument(spoofedDto as never, 'org_own'); + + expect(soaService.createDocument).toHaveBeenCalledWith( + expect.objectContaining({ organizationId: 'org_own' }), + ); + }); }); describe('ensureSetup', () => { @@ -146,6 +217,19 @@ describe('SOAController', () => { expect(soaService.ensureSetup).toHaveBeenCalledWith(dto); expect(result).toEqual(setupResult); }); + + it('should overwrite a body-supplied organizationId with the session organizationId', async () => { + const spoofedDto = { ...dto, organizationId: 'org_victim' }; + mockSOAService.ensureSetup.mockResolvedValue({ + document: { id: 'doc_1' }, + }); + + await controller.ensureSetup(spoofedDto as never, 'org_own'); + + expect(soaService.ensureSetup).toHaveBeenCalledWith( + expect.objectContaining({ organizationId: 'org_own' }), + ); + }); }); describe('getSetup', () => { @@ -162,11 +246,26 @@ describe('SOAController', () => { }; mockSOAService.getSetup.mockResolvedValue(setupResult); - const result = await controller.getSetup(dto as never, 'org_123'); + const result = await controller.getSetup(dto, 'org_123'); expect(soaService.getSetup).toHaveBeenCalledWith(dto); expect(result).toEqual(setupResult); }); + + it('should overwrite a body-supplied organizationId with the session organizationId', async () => { + const spoofedDto = { ...dto, organizationId: 'org_victim' }; + mockSOAService.getSetup.mockResolvedValue({ + success: true, + configuration: null, + document: null, + }); + + await controller.getSetup(spoofedDto, 'org_own'); + + expect(soaService.getSetup).toHaveBeenCalledWith( + expect.objectContaining({ organizationId: 'org_own' }), + ); + }); }); describe('approveDocument', () => { @@ -180,7 +279,7 @@ describe('SOAController', () => { mockSOAService.approveDocument.mockResolvedValue(approved); const result = await controller.approveDocument( - dto as never, + dto, 'org_123', mockAuthContext, ); @@ -194,6 +293,18 @@ describe('SOAController', () => { controller.approveDocument(dto as never, 'org_123', noUserAuthContext), ).rejects.toThrow(BadRequestException); }); + + it('should overwrite a body-supplied organizationId with the session organizationId', async () => { + const spoofedDto = { ...dto, organizationId: 'org_victim' }; + mockSOAService.approveDocument.mockResolvedValue({ success: true }); + + await controller.approveDocument(spoofedDto, 'org_own', mockAuthContext); + + expect(soaService.approveDocument).toHaveBeenCalledWith( + expect.objectContaining({ organizationId: 'org_own' }), + 'usr_123', + ); + }); }); describe('declineDocument', () => { @@ -208,7 +319,7 @@ describe('SOAController', () => { mockSOAService.declineDocument.mockResolvedValue(declined); const result = await controller.declineDocument( - dto as never, + dto, 'org_123', mockAuthContext, ); @@ -222,6 +333,18 @@ describe('SOAController', () => { controller.declineDocument(dto as never, 'org_123', noUserAuthContext), ).rejects.toThrow(BadRequestException); }); + + it('should overwrite a body-supplied organizationId with the session organizationId', async () => { + const spoofedDto = { ...dto, organizationId: 'org_victim' }; + mockSOAService.declineDocument.mockResolvedValue({ success: true }); + + await controller.declineDocument(spoofedDto, 'org_own', mockAuthContext); + + expect(soaService.declineDocument).toHaveBeenCalledWith( + expect.objectContaining({ organizationId: 'org_own' }), + 'usr_123', + ); + }); }); describe('submitForApproval', () => { @@ -242,6 +365,17 @@ describe('SOAController', () => { expect(soaService.submitForApproval).toHaveBeenCalledWith(dto); expect(result).toEqual(submitted); }); + + it('should overwrite a body-supplied organizationId with the session organizationId', async () => { + const spoofedDto = { ...dto, organizationId: 'org_victim' }; + mockSOAService.submitForApproval.mockResolvedValue({ success: true }); + + await controller.submitForApproval(spoofedDto as never, 'org_own'); + + expect(soaService.submitForApproval).toHaveBeenCalledWith( + expect.objectContaining({ organizationId: 'org_own' }), + ); + }); }); describe('exportDocument', () => { @@ -278,5 +412,24 @@ describe('SOAController', () => { ); expect(res.send).toHaveBeenCalledWith(fileBuffer); }); + + it('should overwrite a body-supplied organizationId with the session organizationId', async () => { + const spoofedDto = { ...dto, organizationId: 'org_victim' }; + mockSOAService.exportDocument.mockResolvedValue({ + fileBuffer: Buffer.from('pdf-data'), + mimeType: 'application/pdf', + filename: 'soa-export.pdf', + }); + const res = { + setHeader: jest.fn(), + send: jest.fn(), + } as unknown as Response; + + await controller.exportDocument(spoofedDto as never, res, 'org_own'); + + expect(soaService.exportDocument).toHaveBeenCalledWith( + expect.objectContaining({ organizationId: 'org_own' }), + ); + }); }); }); diff --git a/apps/api/src/soa/soa.controller.ts b/apps/api/src/soa/soa.controller.ts index f71d0c2ee3..345dc6e604 100644 --- a/apps/api/src/soa/soa.controller.ts +++ b/apps/api/src/soa/soa.controller.ts @@ -76,6 +76,7 @@ export class SOAController { throw new BadRequestException('User not authenticated'); } + dto.organizationId = organizationId; return this.soaService.saveAnswer(dto, authContext.userId); } @@ -97,6 +98,7 @@ export class SOAController { throw new BadRequestException('User not authenticated'); } + dto.organizationId = organizationId; const userId = authContext.userId; setupSSEHeaders(res); const send = createSafeSSESender(res); @@ -319,6 +321,7 @@ export class SOAController { @Body() dto: CreateSOADocumentDto, @OrganizationId() organizationId: string, ) { + dto.organizationId = organizationId; return this.soaService.createDocument(dto); } @@ -334,6 +337,7 @@ export class SOAController { @Body() dto: EnsureSOASetupDto, @OrganizationId() organizationId: string, ) { + dto.organizationId = organizationId; return this.soaService.ensureSetup(dto); } @@ -372,6 +376,7 @@ export class SOAController { throw new BadRequestException('User not authenticated'); } + dto.organizationId = organizationId; return this.soaService.approveDocument(dto, authContext.userId); } @@ -392,6 +397,7 @@ export class SOAController { throw new BadRequestException('User not authenticated'); } + dto.organizationId = organizationId; return this.soaService.declineDocument(dto, authContext.userId); } @@ -407,6 +413,7 @@ export class SOAController { @Body() dto: SubmitSOAForApprovalDto, @OrganizationId() organizationId: string, ) { + dto.organizationId = organizationId; return this.soaService.submitForApproval(dto); } From 02bad959dfee147e3267dac72f3c0c5d6c4746a9 Mon Sep 17 00:00:00 2001 From: Dennis Lysenko Date: Mon, 21 Sep 2026 15:11:24 -0400 Subject: [PATCH 2/6] fix(api): scope task automations to task and organization automationId lookups are now verified against the task in the URL and the caller's organization via a shared verifyAutomationAccess helper, so an automationId from another org's task 404s on read, update, delete, runs, versions, and publish instead of leaking or mutating it. Refs GH-046 --- .../automations/automations.controller.ts | 42 +++- .../automations/automations.service.spec.ts | 196 ++++++++++++++++-- .../tasks/automations/automations.service.ts | 127 +++++++++--- 3 files changed, 318 insertions(+), 47 deletions(-) diff --git a/apps/api/src/tasks/automations/automations.controller.ts b/apps/api/src/tasks/automations/automations.controller.ts index c07c3f0434..887f7a810d 100644 --- a/apps/api/src/tasks/automations/automations.controller.ts +++ b/apps/api/src/tasks/automations/automations.controller.ts @@ -92,7 +92,11 @@ export class AutomationsController { // Verify task access first await this.tasksService.verifyTaskAccess(organizationId, taskId); - return this.automationsService.findById(automationId); + return this.automationsService.findById({ + organizationId, + taskId, + automationId, + }); } @Post() @@ -143,7 +147,12 @@ export class AutomationsController { // Verify task access first await this.tasksService.verifyTaskAccess(organizationId, taskId); - return this.automationsService.update(automationId, updateAutomationDto); + return this.automationsService.update({ + organizationId, + taskId, + automationId, + updateAutomationDto, + }); } @Delete(':automationId') @@ -174,7 +183,11 @@ export class AutomationsController { // Verify task access first await this.tasksService.verifyTaskAccess(organizationId, taskId); - return this.automationsService.delete(automationId); + return this.automationsService.delete({ + organizationId, + taskId, + automationId, + }); } @Get(':automationId/runs') @@ -192,7 +205,11 @@ export class AutomationsController { @Param('automationId') automationId: string, ) { await this.tasksService.verifyTaskAccess(organizationId, taskId); - return this.automationsService.findRunsByAutomationId(automationId); + return this.automationsService.findRunsByAutomationId({ + organizationId, + taskId, + automationId, + }); } @Get(':automationId/versions') @@ -243,11 +260,13 @@ export class AutomationsController { await this.tasksService.verifyTaskAccess(organizationId, taskId); const parsedLimit = limit ? parseInt(limit) : undefined; const parsedOffset = offset ? parseInt(offset) : undefined; - return this.automationsService.listVersions( + return this.automationsService.listVersions({ + organizationId, + taskId, automationId, - parsedLimit, - parsedOffset, - ); + limit: parsedLimit, + offset: parsedOffset, + }); } @Post(':automationId/versions') @@ -265,7 +284,12 @@ export class AutomationsController { @Body() body: CreateVersionDto, ) { await this.tasksService.verifyTaskAccess(organizationId, taskId); - return this.automationsService.createVersion(automationId, body); + return this.automationsService.createVersion({ + organizationId, + taskId, + automationId, + data: body, + }); } // ==================== AUTOMATION RUNS (per task) ==================== diff --git a/apps/api/src/tasks/automations/automations.service.spec.ts b/apps/api/src/tasks/automations/automations.service.spec.ts index f7a2746ae2..5201b2550f 100644 --- a/apps/api/src/tasks/automations/automations.service.spec.ts +++ b/apps/api/src/tasks/automations/automations.service.spec.ts @@ -16,8 +16,13 @@ jest.mock('@db', () => { return { db: { $transaction: jest.fn(), - evidenceAutomationVersion: { create: jest.fn() }, - evidenceAutomation: { update: jest.fn() }, + evidenceAutomationVersion: { create: jest.fn(), findMany: jest.fn() }, + evidenceAutomation: { + findFirst: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + }, + evidenceAutomationRun: { findMany: jest.fn() }, }, Prisma: { PrismaClientKnownRequestError }, }; @@ -32,20 +37,178 @@ const prismaError = (code: string) => clientVersion: '5.0.0', }); +describe('AutomationsService — cross-tenant automation scoping', () => { + let service: AutomationsService; + const scope = { + organizationId: 'org_1', + taskId: 'tsk_1', + automationId: 'aut_1', + }; + const owningAutomation = { id: 'aut_1', taskId: 'tsk_1', name: 'Automation' }; + + beforeEach(() => { + jest.clearAllMocks(); + service = new AutomationsService(); + }); + + describe('findById', () => { + it('returns the automation when it belongs to the task/organization', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue( + owningAutomation, + ); + + const result = await service.findById(scope); + + expect(result).toEqual({ success: true, automation: owningAutomation }); + expect(db.evidenceAutomation.findFirst).toHaveBeenCalledWith({ + where: { + id: scope.automationId, + taskId: scope.taskId, + task: { organizationId: scope.organizationId }, + }, + }); + }); + + it('404s when the automation belongs to a different task/organization', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue(null); + + await expect(service.findById(scope)).rejects.toBeInstanceOf( + NotFoundException, + ); + }); + }); + + describe('update', () => { + const updateAutomationDto = { name: 'New name' }; + + it('updates the automation when scoped ownership checks out', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue( + owningAutomation, + ); + (db.evidenceAutomation.update as jest.Mock).mockResolvedValue({ + id: 'aut_1', + name: 'New name', + description: null, + }); + + const result = await service.update({ ...scope, updateAutomationDto }); + + expect(result.success).toBe(true); + expect(db.evidenceAutomation.update).toHaveBeenCalled(); + }); + + it('404s instead of updating when the automation belongs to another org', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue(null); + + await expect( + service.update({ ...scope, updateAutomationDto }), + ).rejects.toBeInstanceOf(NotFoundException); + expect(db.evidenceAutomation.update).not.toHaveBeenCalled(); + }); + }); + + describe('delete', () => { + it('deletes the automation when scoped ownership checks out', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue( + owningAutomation, + ); + (db.evidenceAutomation.delete as jest.Mock).mockResolvedValue( + owningAutomation, + ); + + const result = await service.delete(scope); + + expect(result).toEqual({ + success: true, + message: 'Automation deleted successfully', + }); + }); + + it('404s instead of deleting when the automation belongs to another org', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue(null); + + await expect(service.delete(scope)).rejects.toBeInstanceOf( + NotFoundException, + ); + expect(db.evidenceAutomation.delete).not.toHaveBeenCalled(); + }); + }); + + describe('findRunsByAutomationId', () => { + it('returns runs when scoped ownership checks out', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue( + owningAutomation, + ); + (db.evidenceAutomationRun.findMany as jest.Mock).mockResolvedValue([ + { id: 'ear_1' }, + ]); + + const result = await service.findRunsByAutomationId(scope); + + expect(result).toEqual([{ id: 'ear_1' }]); + }); + + it('404s instead of listing runs when the automation belongs to another org', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue(null); + + await expect( + service.findRunsByAutomationId(scope), + ).rejects.toBeInstanceOf(NotFoundException); + expect(db.evidenceAutomationRun.findMany).not.toHaveBeenCalled(); + }); + }); + + describe('listVersions', () => { + it('returns versions when scoped ownership checks out', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue( + owningAutomation, + ); + (db.evidenceAutomationVersion.findMany as jest.Mock).mockResolvedValue([ + { id: 'eav_1' }, + ]); + + const result = await service.listVersions(scope); + + expect(result).toEqual({ success: true, versions: [{ id: 'eav_1' }] }); + }); + + it('404s instead of listing versions when the automation belongs to another org', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue(null); + + await expect(service.listVersions(scope)).rejects.toBeInstanceOf( + NotFoundException, + ); + expect(db.evidenceAutomationVersion.findMany).not.toHaveBeenCalled(); + }); + }); +}); + describe('AutomationsService.createVersion — error mapping', () => { let service: AutomationsService; - const input = { version: 1, scriptKey: 'org_1/tsk_1/aut_1.v1.js' }; + const scope = { + organizationId: 'org_1', + taskId: 'tsk_1', + automationId: 'aut_1', + }; + const data = { version: 1, scriptKey: 'org_1/tsk_1/aut_1.v1.js' }; + const owningAutomation = { id: 'aut_1', taskId: 'tsk_1', name: 'Automation' }; beforeEach(() => { jest.clearAllMocks(); service = new AutomationsService(); + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue( + owningAutomation, + ); }); it('records the version and returns it on success', async () => { - const created = { id: 'eav_1', version: 1, scriptKey: input.scriptKey }; - (db.$transaction as jest.Mock).mockResolvedValue([created, { id: 'aut_1' }]); + const created = { id: 'eav_1', version: 1, scriptKey: data.scriptKey }; + (db.$transaction as jest.Mock).mockResolvedValue([ + created, + { id: 'aut_1' }, + ]); - const result = await service.createVersion('aut_1', input); + const result = await service.createVersion({ ...scope, data }); expect(result).toEqual({ success: true, version: created }); }); @@ -53,16 +216,16 @@ describe('AutomationsService.createVersion — error mapping', () => { it('maps a duplicate version (P2002) to a 409 ConflictException', async () => { (db.$transaction as jest.Mock).mockRejectedValue(prismaError('P2002')); - await expect(service.createVersion('aut_1', input)).rejects.toBeInstanceOf( - ConflictException, - ); + await expect( + service.createVersion({ ...scope, data }), + ).rejects.toBeInstanceOf(ConflictException); }); it('maps a missing automation (P2003 FK violation) to a 404 NotFoundException', async () => { (db.$transaction as jest.Mock).mockRejectedValue(prismaError('P2003')); await expect( - service.createVersion('missing', input), + service.createVersion({ ...scope, data }), ).rejects.toBeInstanceOf(NotFoundException); }); @@ -70,7 +233,7 @@ describe('AutomationsService.createVersion — error mapping', () => { (db.$transaction as jest.Mock).mockRejectedValue(prismaError('P2025')); await expect( - service.createVersion('missing', input), + service.createVersion({ ...scope, data }), ).rejects.toBeInstanceOf(NotFoundException); }); @@ -78,6 +241,15 @@ describe('AutomationsService.createVersion — error mapping', () => { const boom = new Error('db exploded'); (db.$transaction as jest.Mock).mockRejectedValue(boom); - await expect(service.createVersion('aut_1', input)).rejects.toBe(boom); + await expect(service.createVersion({ ...scope, data })).rejects.toBe(boom); + }); + + it('404s before attempting the transaction when the automation belongs to another org', async () => { + (db.evidenceAutomation.findFirst as jest.Mock).mockResolvedValue(null); + + await expect( + service.createVersion({ ...scope, data }), + ).rejects.toBeInstanceOf(NotFoundException); + expect(db.$transaction).not.toHaveBeenCalled(); }); }); diff --git a/apps/api/src/tasks/automations/automations.service.ts b/apps/api/src/tasks/automations/automations.service.ts index 1e8bf00254..8752f2271f 100644 --- a/apps/api/src/tasks/automations/automations.service.ts +++ b/apps/api/src/tasks/automations/automations.service.ts @@ -33,10 +33,26 @@ export class AutomationsService { }; } - async findById(automationId: string) { + /** + * Confirms the automation exists, belongs to `taskId`, and that task + * belongs to `organizationId`. The controller only verifies the task in + * the URL — this closes the gap where `automationId` could otherwise + * reference a different task/org entirely. + */ + private async verifyAutomationAccess({ + organizationId, + taskId, + automationId, + }: { + organizationId: string; + taskId: string; + automationId: string; + }) { const automation = await db.evidenceAutomation.findFirst({ where: { id: automationId, + taskId, + task: { organizationId }, }, }); @@ -44,6 +60,24 @@ export class AutomationsService { throw new NotFoundException('Automation not found'); } + return automation; + } + + async findById({ + organizationId, + taskId, + automationId, + }: { + organizationId: string; + taskId: string; + automationId: string; + }) { + const automation = await this.verifyAutomationAccess({ + organizationId, + taskId, + automationId, + }); + return { success: true, automation, @@ -80,17 +114,19 @@ export class AutomationsService { }; } - async update(automationId: string, updateAutomationDto: UpdateAutomationDto) { - // Verify automation exists and belongs to organization - const existingAutomation = await db.evidenceAutomation.findFirst({ - where: { - id: automationId, - }, - }); - - if (!existingAutomation) { - throw new NotFoundException('Automation not found'); - } + async update({ + organizationId, + taskId, + automationId, + updateAutomationDto, + }: { + organizationId: string; + taskId: string; + automationId: string; + updateAutomationDto: UpdateAutomationDto; + }) { + // Verify automation exists and belongs to the task/organization + await this.verifyAutomationAccess({ organizationId, taskId, automationId }); const { scheduleFrequency, ...rest } = updateAutomationDto; @@ -115,17 +151,17 @@ export class AutomationsService { }; } - async delete(automationId: string) { - // Verify automation exists and belongs to organization - const existingAutomation = await db.evidenceAutomation.findFirst({ - where: { - id: automationId, - }, - }); - - if (!existingAutomation) { - throw new NotFoundException('Automation not found'); - } + async delete({ + organizationId, + taskId, + automationId, + }: { + organizationId: string; + taskId: string; + automationId: string; + }) { + // Verify automation exists and belongs to the task/organization + await this.verifyAutomationAccess({ organizationId, taskId, automationId }); // Delete the automation await db.evidenceAutomation.delete({ @@ -140,7 +176,20 @@ export class AutomationsService { }; } - async createVersion(automationId: string, data: CreateVersionDto) { + async createVersion({ + organizationId, + taskId, + automationId, + data, + }: { + organizationId: string; + taskId: string; + automationId: string; + data: CreateVersionDto; + }) { + // Verify automation exists and belongs to the task/organization + await this.verifyAutomationAccess({ organizationId, taskId, automationId }); + try { const [version] = await db.$transaction([ db.evidenceAutomationVersion.create({ @@ -176,7 +225,18 @@ export class AutomationsService { } } - async findRunsByAutomationId(automationId: string) { + async findRunsByAutomationId({ + organizationId, + taskId, + automationId, + }: { + organizationId: string; + taskId: string; + automationId: string; + }) { + // Verify automation exists and belongs to the task/organization + await this.verifyAutomationAccess({ organizationId, taskId, automationId }); + const runs = await db.evidenceAutomationRun.findMany({ where: { evidenceAutomationId: automationId, @@ -194,7 +254,22 @@ export class AutomationsService { return runs; } - async listVersions(automationId: string, limit?: number, offset?: number) { + async listVersions({ + organizationId, + taskId, + automationId, + limit, + offset, + }: { + organizationId: string; + taskId: string; + automationId: string; + limit?: number; + offset?: number; + }) { + // Verify automation exists and belongs to the task/organization + await this.verifyAutomationAccess({ organizationId, taskId, automationId }); + const versions = await db.evidenceAutomationVersion.findMany({ where: { evidenceAutomationId: automationId, From d143acf30705f91cb06d52c6f5b9005c81adc4a8 Mon Sep 17 00:00:00 2001 From: Dennis Lysenko Date: Mon, 21 Sep 2026 15:11:30 -0400 Subject: [PATCH 3/6] fix(api): harden public trust-portal access endpoints reclaimAccess no longer returns the access link/token in the response body and returns an identical generic message whether or not a grant exists, removing the unauthenticated token disclosure and the email-enumeration oracle; the link is only emailed to the requester. findPublishedTrustByRouteId no longer auto-creates a published Trust row or flips drafts to published: public endpoints only resolve rows that are already published and 404 otherwise, so an unauthenticated caller can no longer force-publish an organization's trust portal. Refs GH-042, GH-272 --- .../trust-access.controller.spec.ts | 31 +++- .../trust-portal/trust-access.controller.ts | 3 +- .../trust-portal/trust-access.service.spec.ts | 135 +++++++++++++++++- .../src/trust-portal/trust-access.service.ts | 54 +++---- 4 files changed, 175 insertions(+), 48 deletions(-) diff --git a/apps/api/src/trust-portal/trust-access.controller.spec.ts b/apps/api/src/trust-portal/trust-access.controller.spec.ts index cb83ddaa82..1d41227ab0 100644 --- a/apps/api/src/trust-portal/trust-access.controller.spec.ts +++ b/apps/api/src/trust-portal/trust-access.controller.spec.ts @@ -280,7 +280,7 @@ describe('TrustAccessController', () => { const req = mockRequest(); mockService.signNda.mockResolvedValue({ success: true }); - const result = await controller.signNda('token_abc', dto as any, req); + const result = await controller.signNda('token_abc', dto, req); expect(result).toEqual({ success: true }); expect(service.signNda).toHaveBeenCalledWith( @@ -326,17 +326,27 @@ describe('TrustAccessController', () => { }); describe('reclaimAccess', () => { + // GH-042: the response body is a generic message regardless of whether a + // grant exists, so the controller test only needs to prove it forwards + // params and returns the service's response verbatim (no accessLink or + // token added or stripped in the controller layer). + const GENERIC_RESPONSE = { + message: + 'If an active access grant exists for this email, an access link has been sent.', + }; + it('should call service.reclaimAccess with friendlyUrl, email, and query', async () => { const dto = { email: 'user@example.com' }; - mockService.reclaimAccess.mockResolvedValue({ success: true }); + mockService.reclaimAccess.mockResolvedValue(GENERIC_RESPONSE); const result = await controller.reclaimAccess( 'my-portal', - dto as any, + dto, 'security-questionnaire', ); - expect(result).toEqual({ success: true }); + expect(result).toEqual(GENERIC_RESPONSE); + expect(result).not.toHaveProperty('accessLink'); expect(service.reclaimAccess).toHaveBeenCalledWith( 'my-portal', 'user@example.com', @@ -346,9 +356,9 @@ describe('TrustAccessController', () => { it('should pass undefined query when not provided', async () => { const dto = { email: 'user@example.com' }; - mockService.reclaimAccess.mockResolvedValue({ success: true }); + mockService.reclaimAccess.mockResolvedValue(GENERIC_RESPONSE); - await controller.reclaimAccess('my-portal', dto as any); + await controller.reclaimAccess('my-portal', dto); expect(service.reclaimAccess).toHaveBeenCalledWith( 'my-portal', @@ -356,6 +366,15 @@ describe('TrustAccessController', () => { undefined, ); }); + + it('returns the same generic response whether or not a grant exists (no enumeration signal)', async () => { + const dto = { email: 'nobody@example.com' }; + mockService.reclaimAccess.mockResolvedValue(GENERIC_RESPONSE); + + const result = await controller.reclaimAccess('my-portal', dto); + + expect(result).toEqual(GENERIC_RESPONSE); + }); }); describe('getGrantByAccessToken', () => { diff --git a/apps/api/src/trust-portal/trust-access.controller.ts b/apps/api/src/trust-portal/trust-access.controller.ts index 8db04c19b2..a7ea1f105f 100644 --- a/apps/api/src/trust-portal/trust-access.controller.ts +++ b/apps/api/src/trust-portal/trust-access.controller.ts @@ -378,7 +378,8 @@ export class TrustAccessController { }) @ApiResponse({ status: HttpStatus.OK, - description: 'Access link sent to email', + description: + 'Generic confirmation message, identical whether or not an active access grant exists for the email (no access link or token is returned).', }) async reclaimAccess( // Note: friendlyUrl can be either the custom friendly URL or the organization ID diff --git a/apps/api/src/trust-portal/trust-access.service.spec.ts b/apps/api/src/trust-portal/trust-access.service.spec.ts index 0a812b821d..73780ee2fc 100644 --- a/apps/api/src/trust-portal/trust-access.service.spec.ts +++ b/apps/api/src/trust-portal/trust-access.service.spec.ts @@ -8,7 +8,10 @@ jest.mock('@db', () => ({ db: { trust: { findUnique: jest.fn(), + findFirst: jest.fn(), upsert: jest.fn(), + create: jest.fn(), + update: jest.fn(), }, trustNDAAgreement: { findUnique: jest.fn(), @@ -64,7 +67,10 @@ jest.mock('../app/s3', () => ({ const mockDb = db as unknown as { trust: { findUnique: jest.Mock; + findFirst: jest.Mock; upsert: jest.Mock; + create: jest.Mock; + update: jest.Mock; }; trustNDAAgreement: { findUnique: jest.Mock; @@ -547,7 +553,91 @@ describe('TrustAccessService signNda NDA copy', () => { }); }); -describe('TrustAccessService reclaimAccess token rotation', () => { +describe('TrustAccessService findPublishedTrustByRouteId (GH-272)', () => { + const service = new TrustAccessService( + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + ); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('resolves a published trust row by friendlyUrl', async () => { + mockDb.trust.findFirst.mockResolvedValueOnce({ + organizationId: 'org_1', + friendlyUrl: 'acme-security', + status: 'published', + organization: { name: 'Acme Security' }, + }); + + const result = await (service as any).findPublishedTrustByRouteId( + 'acme-security', + ); + + expect(mockDb.trust.findFirst).toHaveBeenCalledWith({ + where: { friendlyUrl: 'acme-security', status: 'published' }, + include: { organization: true }, + }); + expect(result.organizationId).toBe('org_1'); + }); + + it('falls back to organizationId when no friendlyUrl match exists', async () => { + mockDb.trust.findFirst.mockResolvedValueOnce(null).mockResolvedValueOnce({ + organizationId: 'org_1', + friendlyUrl: 'org_1', + status: 'published', + organization: { name: 'Acme Security' }, + }); + + const result = await (service as any).findPublishedTrustByRouteId('org_1'); + + expect(mockDb.trust.findFirst).toHaveBeenNthCalledWith(2, { + where: { organizationId: 'org_1', status: 'published' }, + include: { organization: true }, + }); + expect(result.organizationId).toBe('org_1'); + }); + + it('404s when no trust row exists for the id, without auto-creating one', async () => { + mockDb.trust.findFirst.mockResolvedValue(null); + + await expect( + (service as any).findPublishedTrustByRouteId('unknown-id'), + ).rejects.toThrow('Trust site not found'); + + expect(mockDb.trust.create).not.toHaveBeenCalled(); + expect(mockDb.trust.update).not.toHaveBeenCalled(); + }); + + it('404s for a draft trust row instead of silently auto-publishing it', async () => { + // A draft row never matches the status: 'published' filter, so both the + // friendlyUrl and organizationId lookups come back empty even though a + // row exists in the table. + mockDb.trust.findFirst.mockResolvedValue(null); + + await expect( + (service as any).findPublishedTrustByRouteId('draft-portal'), + ).rejects.toThrow('Trust site not found'); + + expect(mockDb.trust.findFirst).toHaveBeenCalledWith({ + where: { friendlyUrl: 'draft-portal', status: 'published' }, + include: { organization: true }, + }); + expect(mockDb.trust.update).not.toHaveBeenCalled(); + expect(mockDb.trust.create).not.toHaveBeenCalled(); + }); +}); + +describe('TrustAccessService reclaimAccess (GH-042)', () => { + const GENERIC_RESPONSE = { + message: + 'If an active access grant exists for this email, an access link has been sent.', + }; + const emailService = { sendAccessReclaimEmail: jest.fn(), }; @@ -564,7 +654,7 @@ describe('TrustAccessService reclaimAccess token rotation', () => { beforeEach(() => { jest.clearAllMocks(); - mockDb.trust.findUnique.mockResolvedValue({ + mockDb.trust.findFirst.mockResolvedValue({ organizationId: 'org_1', friendlyUrl: 'acme-security', status: 'published', @@ -594,6 +684,43 @@ describe('TrustAccessService reclaimAccess token rotation', () => { data: expect.objectContaining({ accessTokenExpiresAt: grantExpiresAt }), }); }); + + it('returns only a generic message, with no accessLink or token, when a grant exists', async () => { + mockDb.trustAccessGrant.findFirst.mockResolvedValue({ + id: 'tag_1', + subjectEmail: 'chang.liu@client.com', + status: 'active', + expiresAt: new Date(Date.now() + 20 * 24 * 60 * 60 * 1000), + accessToken: 'existing-token', + accessTokenExpiresAt: new Date(Date.now() + 20 * 24 * 60 * 60 * 1000), + accessRequest: { + name: 'Chang Liu', + organization: { name: 'Acme Security' }, + }, + ndaAgreement: null, + }); + + const result = await service.reclaimAccess( + 'acme-security', + 'chang.liu@client.com', + ); + + expect(result).toEqual(GENERIC_RESPONSE); + expect(result).not.toHaveProperty('accessLink'); + expect(emailService.sendAccessReclaimEmail).toHaveBeenCalledTimes(1); + }); + + it('returns the same generic message and does not throw when no grant exists', async () => { + mockDb.trustAccessGrant.findFirst.mockResolvedValue(null); + + const result = await service.reclaimAccess( + 'acme-security', + 'nobody@client.com', + ); + + expect(result).toEqual(GENERIC_RESPONSE); + expect(emailService.sendAccessReclaimEmail).not.toHaveBeenCalled(); + }); }); describe('TrustAccessService access request notification', () => { @@ -619,7 +746,9 @@ describe('TrustAccessService access request notification', () => { it('points the review button at the access requests page, not the trust overview', async () => { // contactEmail present -> single recipient, no member fallback lookup. - mockDb.trust.findUnique.mockResolvedValue({ contactEmail: 'owner@acme.com' }); + mockDb.trust.findUnique.mockResolvedValue({ + contactEmail: 'owner@acme.com', + }); const dto: CreateAccessRequestDto = { name: 'Jane Doe', diff --git a/apps/api/src/trust-portal/trust-access.service.ts b/apps/api/src/trust-portal/trust-access.service.ts index 939c43189d..3133d854f1 100644 --- a/apps/api/src/trust-portal/trust-access.service.ts +++ b/apps/api/src/trust-portal/trust-access.service.ts @@ -186,47 +186,21 @@ export class TrustAccessService { private async findPublishedTrustByRouteId(id: string) { // First, try treating `id` as the existing friendlyUrl. - let trust = await db.trust.findUnique({ - where: { friendlyUrl: id }, + let trust = await db.trust.findFirst({ + where: { friendlyUrl: id, status: 'published' }, include: { organization: true }, }); // If none found, fall back to treating `id` as organizationId. if (!trust) { trust = await db.trust.findFirst({ - where: { organizationId: id }, + where: { organizationId: id, status: 'published' }, include: { organization: true }, }); } - // If still no trust record but we have an organization, auto-create it if (!trust) { - const organization = await db.organization.findUnique({ - where: { id }, - }); - - if (!organization) { - throw new NotFoundException('Trust site not found'); - } - - // Auto-create trust record with organizationId as friendlyUrl - trust = await db.trust.create({ - data: { - organizationId: id, - friendlyUrl: id, - status: 'published', - }, - include: { organization: true }, - }); - } - - // Ensure the trust portal is published (auto-publish if draft) - if (trust.status !== 'published') { - trust = await db.trust.update({ - where: { organizationId: trust.organizationId }, - data: { status: 'published' }, - include: { organization: true }, - }); + throw new NotFoundException('Trust site not found'); } return trust; @@ -1408,6 +1382,11 @@ export class TrustAccessService { }; } + private readonly RECLAIM_GENERIC_RESPONSE = { + message: + 'If an active access grant exists for this email, an access link has been sent.', + }; + async reclaimAccess(id: string, email: string, query?: string) { const trust = await this.findPublishedTrustByRouteId(id); @@ -1432,10 +1411,10 @@ export class TrustAccessService { }, }); + // Return the same generic response whether or not a grant exists, so the + // response body can't be used to enumerate registered emails. if (!grant) { - throw new NotFoundException( - 'No active access grant found for this email', - ); + return this.RECLAIM_GENERIC_RESPONSE; } let accessToken = grant.accessToken; @@ -1480,11 +1459,10 @@ export class TrustAccessService { expiresAt: grant.expiresAt, }); - return { - message: 'Access link sent to your email', - accessLink, - expiresAt: accessTokenExpiresAt, - }; + // Never return the access link or token in the response body: it must + // stay identical to the no-grant case to avoid an email-enumeration + // oracle. + return this.RECLAIM_GENERIC_RESPONSE; } async getGrantByAccessToken(token: string) { From 3a1421bb1c665f1721ea33bba5f69333346b20f8 Mon Sep 17 00:00:00 2001 From: Dennis Lysenko Date: Mon, 21 Sep 2026 15:11:39 -0400 Subject: [PATCH 4/6] fix(app): require session and org match in task-automation actions Every exported server action now resolves the caller's session and active organization and fails closed when unauthenticated. Actions that take orgId must match the session's active org; S3 keys must be prefixed with the active org; automationId-only actions verify ownership through the automation's task before proxying to the enterprise API. Refs GH-052 --- .../actions/task-automation-actions.ts | 146 ++++++++++++++++-- 1 file changed, 133 insertions(+), 13 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/automation/[automationId]/actions/task-automation-actions.ts b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/automation/[automationId]/actions/task-automation-actions.ts index e145e1a0f1..429ab9be5d 100644 --- a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/automation/[automationId]/actions/task-automation-actions.ts +++ b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/automation/[automationId]/actions/task-automation-actions.ts @@ -5,6 +5,8 @@ * These actions securely call the enterprise API with server-side license key */ +import { auth } from '@/utils/auth'; +import { db } from '@db/server'; import { revalidatePath } from 'next/cache'; import { headers } from 'next/headers'; @@ -100,6 +102,38 @@ async function callEnterpriseApi( return result.data || (result as T); } +/** + * Resolve the caller's session and active organization. Every exported + * action in this file is a publicly-invocable Next.js Server Action RPC, so + * this MUST be called first in each one and the caller MUST fail closed + * (never proceed) when it returns null. + */ +async function getActiveOrganizationId(): Promise { + const session = await auth.api.getSession({ headers: await headers() }); + + return session?.session.activeOrganizationId ?? null; +} + +/** + * Check that an automation (identified only by its own id, with no orgId + * supplied by the caller) belongs to the given organization, by following + * automation -> task -> organizationId. + */ +async function isAutomationInOrganization({ + automationId, + organizationId, +}: { + automationId: string; + organizationId: string; +}): Promise { + const automation = await db.evidenceAutomation.findUnique({ + where: { id: automationId }, + select: { task: { select: { organizationId: true } } }, + }); + + return automation?.task.organizationId === organizationId; +} + /** * Revalidate current path */ @@ -120,6 +154,11 @@ export async function uploadAutomationScript(data: { type?: string; }) { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId || activeOrganizationId !== data.orgId) { + return { success: false, error: 'Unauthorized' }; + } + const result = await callEnterpriseApi('/api/tasks-automations/s3/upload', { method: 'POST', body: data, @@ -140,6 +179,15 @@ export async function uploadAutomationScript(data: { */ export async function getAutomationScript(key: string) { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId) { + return { success: false, error: 'Unauthorized' }; + } + + if (!key.startsWith(`${activeOrganizationId}/`) || key.includes('..')) { + return { success: false, error: 'Unauthorized' }; + } + const result = await callEnterpriseApi('/api/tasks-automations/s3/get', { params: { key }, }); @@ -158,6 +206,11 @@ export async function getAutomationScript(key: string) { */ export async function listAutomationScripts(orgId: string) { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId || activeOrganizationId !== orgId) { + return { success: false, error: 'Unauthorized' }; + } + const result = await callEnterpriseApi('/api/tasks-automations/s3/list', { params: { orgId }, }); @@ -197,10 +250,18 @@ export async function executeAutomationScript(data: { version?: number; // Optional: test specific version }) { try { - const result = await callEnterpriseApi<{ runId: string }>('/api/tasks-automations/trigger/execute', { - method: 'POST', - body: data, - }); + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId || activeOrganizationId !== data.orgId) { + return { success: false, error: 'Unauthorized' }; + } + + const result = await callEnterpriseApi<{ runId: string }>( + '/api/tasks-automations/trigger/execute', + { + method: 'POST', + body: data, + }, + ); // Don't revalidate - causes page refresh. Test results are handled via polling/state. return { success: true, data: result }; @@ -226,6 +287,11 @@ export async function executeAutomationScript(data: { */ export async function analyzeAutomationWorkflow(scriptContent: string) { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId) { + return { success: false, error: 'Unauthorized' }; + } + const result = await callEnterpriseApi('/api/tasks-automations/workflow/analyze', { method: 'POST', body: { scriptContent }, @@ -249,8 +315,17 @@ export async function analyzeAutomationWorkflow(scriptContent: string) { } } +// NOTE: `runId` is an enterprise/trigger run id with no reliable local +// ownership mapping (it is not stored against an organization in this app's +// database). We can only require an authenticated session here, not verify +// that the run belongs to the caller's organization. export const getAutomationRunStatus = async (runId: string) => { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId) { + return { success: false, error: 'Unauthorized' }; + } + const result = await callEnterpriseApi( `/api/tasks-automations/runs/${encodeURIComponent(runId)}`, {}, @@ -282,6 +357,19 @@ export const getAutomationRunStatus = async (runId: string) => { */ export async function loadChatHistory(automationId: string, offset = 0, limit = 50) { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId) { + return { success: false, error: 'Unauthorized' }; + } + + const belongsToOrg = await isAutomationInOrganization({ + automationId, + organizationId: activeOrganizationId, + }); + if (!belongsToOrg) { + return { success: false, error: 'Unauthorized' }; + } + const response = await callEnterpriseApi<{ messages: any[]; total: number; @@ -313,6 +401,19 @@ export async function loadChatHistory(automationId: string, offset = 0, limit = */ export async function saveChatHistory(automationId: string, messages: any[]) { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId) { + return { success: false, error: 'Unauthorized' }; + } + + const belongsToOrg = await isAutomationInOrganization({ + automationId, + organizationId: activeOrganizationId, + }); + if (!belongsToOrg) { + return { success: false, error: 'Unauthorized' }; + } + await callEnterpriseApi('/api/tasks-automations/chat/save', { method: 'POST', body: { @@ -343,6 +444,11 @@ export async function publishAutomation( changelog?: string, ) { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId || activeOrganizationId !== orgId) { + return { success: false, error: 'Unauthorized' }; + } + // Call enterprise API to copy draft → versioned S3 key const response = await callEnterpriseApi<{ success: boolean; @@ -372,7 +478,8 @@ export async function publishAutomation( }, ); - const versionData = versionRes.data as { success: boolean; version: { version: number } } | undefined; + const versionData = versionRes.data as + { success: boolean; version: { version: number } } | undefined; return { success: true, version: versionData?.version, @@ -396,6 +503,11 @@ export async function restoreVersion( version: number, ) { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId || activeOrganizationId !== orgId) { + return { success: false, error: 'Unauthorized' }; + } + const response = await callEnterpriseApi<{ success: boolean }>( '/api/tasks-automations/restore-version', { @@ -435,11 +547,15 @@ export async function updateEvaluationCriteria( evaluationCriteria: string, ) { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId) { + return { success: false, error: 'Unauthorized' }; + } + const { serverApi } = await import('@/lib/api-server'); - const response = await serverApi.patch( - `/v1/tasks/${taskId}/automations/${automationId}`, - { evaluationCriteria }, - ); + const response = await serverApi.patch(`/v1/tasks/${taskId}/automations/${automationId}`, { + evaluationCriteria, + }); if (response.error) throw new Error(response.error); return { success: true }; } catch (error) { @@ -461,11 +577,15 @@ export async function toggleAutomationEnabled( isEnabled: boolean, ) { try { + const activeOrganizationId = await getActiveOrganizationId(); + if (!activeOrganizationId) { + return { success: false, error: 'Unauthorized' }; + } + const { serverApi } = await import('@/lib/api-server'); - const response = await serverApi.patch( - `/v1/tasks/${taskId}/automations/${automationId}`, - { isEnabled }, - ); + const response = await serverApi.patch(`/v1/tasks/${taskId}/automations/${automationId}`, { + isEnabled, + }); if (response.error) throw new Error(response.error); return { success: true }; } catch (error) { From 0fc062d1df9e79280c1abd89e1a0a1942b0157e1 Mon Sep 17 00:00:00 2001 From: Dennis Lysenko Date: Mon, 21 Sep 2026 15:11:39 -0400 Subject: [PATCH 5/6] fix(app): require auth and run ownership before minting trigger tokens healAndSetAccessToken and createAccessToken now require a session with an active organization and only mint a Trigger.dev run-read token when the run id is recorded against that organization (onboarding job, knowledge base document, or remediation batch). Refs GH-102 --- .../actions/trigger/heal-access-token.test.ts | 177 ++++++++++++++++++ .../src/actions/trigger/heal-access-token.ts | 56 +++++- 2 files changed, 232 insertions(+), 1 deletion(-) create mode 100644 apps/app/src/actions/trigger/heal-access-token.test.ts diff --git a/apps/app/src/actions/trigger/heal-access-token.test.ts b/apps/app/src/actions/trigger/heal-access-token.test.ts new file mode 100644 index 0000000000..d3bdc8314a --- /dev/null +++ b/apps/app/src/actions/trigger/heal-access-token.test.ts @@ -0,0 +1,177 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockGetSession = vi.fn(); +const mockCreatePublicToken = vi.fn(); +const mockOnboardingFindFirst = vi.fn(); +const mockKnowledgeBaseDocumentFindFirst = vi.fn(); +const mockRemediationBatchFindFirst = vi.fn(); +const mockCookieSet = vi.fn(); + +vi.mock('@/utils/auth', () => ({ + auth: { + api: { + getSession: mockGetSession, + }, + }, +})); + +vi.mock('@trigger.dev/sdk', () => ({ + auth: { + createPublicToken: mockCreatePublicToken, + }, +})); + +vi.mock('@db/server', () => ({ + db: { + onboarding: { findFirst: mockOnboardingFindFirst }, + knowledgeBaseDocument: { findFirst: mockKnowledgeBaseDocumentFindFirst }, + remediationBatch: { findFirst: mockRemediationBatchFindFirst }, + }, +})); + +vi.mock('next/headers', () => ({ + headers: vi.fn(async () => new Headers()), + cookies: vi.fn(async () => ({ set: mockCookieSet })), +})); + +const { createAccessToken, healAndSetAccessToken } = await import('./heal-access-token'); + +const RUN_ID = 'run_123'; +const ORG_ID = 'org_abc'; + +function mockSession(activeOrganizationId: string | null | undefined) { + mockGetSession.mockResolvedValue({ + session: { activeOrganizationId }, + user: { id: 'user_1' }, + }); +} + +function mockOwnership(owner: 'onboarding' | 'knowledgeBaseDocument' | 'remediationBatch' | null) { + mockOnboardingFindFirst.mockResolvedValue( + owner === 'onboarding' ? { organizationId: ORG_ID } : null, + ); + mockKnowledgeBaseDocumentFindFirst.mockResolvedValue( + owner === 'knowledgeBaseDocument' ? { id: 'kbd_1' } : null, + ); + mockRemediationBatchFindFirst.mockResolvedValue( + owner === 'remediationBatch' ? { id: 'rmb_1' } : null, + ); +} + +describe('healAndSetAccessToken', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns null without minting a token when there is no session', async () => { + mockGetSession.mockResolvedValue(null); + + const result = await healAndSetAccessToken(RUN_ID); + + expect(result).toBeNull(); + expect(mockCreatePublicToken).not.toHaveBeenCalled(); + }); + + it('returns null without minting a token when the session has no active organization', async () => { + mockSession(null); + + const result = await healAndSetAccessToken(RUN_ID); + + expect(result).toBeNull(); + expect(mockCreatePublicToken).not.toHaveBeenCalled(); + }); + + it('returns null without minting a token when the run id is not owned by the active organization', async () => { + mockSession(ORG_ID); + mockOwnership(null); + + const result = await healAndSetAccessToken(RUN_ID); + + expect(result).toBeNull(); + expect(mockCreatePublicToken).not.toHaveBeenCalled(); + expect(mockOnboardingFindFirst).toHaveBeenCalledWith({ + where: { organizationId: ORG_ID, triggerJobId: RUN_ID }, + select: { organizationId: true }, + }); + }); + + it('mints and cookies a token when the run id belongs to an onboarding job for the active organization', async () => { + mockSession(ORG_ID); + mockOwnership('onboarding'); + mockCreatePublicToken.mockResolvedValue('public-token'); + + const result = await healAndSetAccessToken(RUN_ID); + + expect(result).toBe('public-token'); + expect(mockCreatePublicToken).toHaveBeenCalledWith({ + scopes: { read: { runs: [RUN_ID] } }, + }); + expect(mockCookieSet).toHaveBeenCalledWith('publicAccessToken', 'public-token'); + }); + + it('mints a token when the run id belongs to a knowledge base document for the active organization', async () => { + mockSession(ORG_ID); + mockOwnership('knowledgeBaseDocument'); + mockCreatePublicToken.mockResolvedValue('public-token'); + + const result = await healAndSetAccessToken(RUN_ID); + + expect(result).toBe('public-token'); + }); + + it('mints a token when the run id belongs to a remediation batch for the active organization', async () => { + mockSession(ORG_ID); + mockOwnership('remediationBatch'); + mockCreatePublicToken.mockResolvedValue('public-token'); + + const result = await healAndSetAccessToken(RUN_ID); + + expect(result).toBe('public-token'); + }); + + it('returns null when the run id is owned by a different organization than the caller is active in', async () => { + mockSession('some-other-org'); + mockOwnership(null); + + const result = await healAndSetAccessToken(RUN_ID); + + expect(result).toBeNull(); + expect(mockCreatePublicToken).not.toHaveBeenCalled(); + }); +}); + +describe('createAccessToken', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns null without minting a token when there is no session', async () => { + mockGetSession.mockResolvedValue(null); + + const result = await createAccessToken(RUN_ID); + + expect(result).toBeNull(); + expect(mockCreatePublicToken).not.toHaveBeenCalled(); + }); + + it('returns null without minting a token when the run id is not owned by the active organization', async () => { + mockSession(ORG_ID); + mockOwnership(null); + + const result = await createAccessToken(RUN_ID); + + expect(result).toBeNull(); + expect(mockCreatePublicToken).not.toHaveBeenCalled(); + }); + + it('mints a token without setting a cookie when the run id is owned by the active organization', async () => { + mockSession(ORG_ID); + mockOwnership('onboarding'); + mockCreatePublicToken.mockResolvedValue('public-token'); + + const result = await createAccessToken(RUN_ID); + + expect(result).toBe('public-token'); + expect(mockCookieSet).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/app/src/actions/trigger/heal-access-token.ts b/apps/app/src/actions/trigger/heal-access-token.ts index 08dfd70bf8..ffb237a0e9 100644 --- a/apps/app/src/actions/trigger/heal-access-token.ts +++ b/apps/app/src/actions/trigger/heal-access-token.ts @@ -1,11 +1,52 @@ 'use server'; +import { auth as appAuth } from '@/utils/auth'; +import { db } from '@db/server'; import { auth } from '@trigger.dev/sdk'; -import { cookies } from 'next/headers'; +import { cookies, headers } from 'next/headers'; + +/** + * Confirms the run id was actually issued for the caller's active + * organization before we mint a token scoped to it. A run id is legitimate + * only if it is recorded on one of the org-scoped records that store + * trigger.dev run ids: an onboarding job, a knowledge base document's + * processing run, or a cloud security remediation batch. + */ +async function isRunOwnedByOrganization(runId: string, organizationId: string): Promise { + const [onboarding, knowledgeBaseDocument, remediationBatch] = await Promise.all([ + db.onboarding.findFirst({ + where: { organizationId, triggerJobId: runId }, + select: { organizationId: true }, + }), + db.knowledgeBaseDocument.findFirst({ + where: { organizationId, triggerRunId: runId }, + select: { id: true }, + }), + db.remediationBatch.findFirst({ + where: { organizationId, triggerRunId: runId }, + select: { id: true }, + }), + ]); + + return onboarding !== null || knowledgeBaseDocument !== null || remediationBatch !== null; +} // Server action that can set cookies (called from client components or forms) export async function healAndSetAccessToken(triggerJobId: string): Promise { try { + const session = await appAuth.api.getSession({ headers: await headers() }); + const organizationId = session?.session.activeOrganizationId; + + if (!organizationId) { + return null; + } + + const isOwned = await isRunOwnedByOrganization(triggerJobId, organizationId); + + if (!isOwned) { + return null; + } + const cookieStore = await cookies(); const token = await auth.createPublicToken({ @@ -28,6 +69,19 @@ export async function healAndSetAccessToken(triggerJobId: string): Promise { try { + const session = await appAuth.api.getSession({ headers: await headers() }); + const organizationId = session?.session.activeOrganizationId; + + if (!organizationId) { + return null; + } + + const isOwned = await isRunOwnedByOrganization(triggerJobId, organizationId); + + if (!isOwned) { + return null; + } + const token = await auth.createPublicToken({ scopes: { read: { From 849a32002e5cfd4e30d9fd6196f89505adaeb92d Mon Sep 17 00:00:00 2001 From: Dennis Lysenko Date: Mon, 21 Sep 2026 15:11:40 -0400 Subject: [PATCH 6/6] ci(device-agent): restrict release workflow to protected branches The device-agent release pipeline runs branch-controlled build scripts with Apple and SSL.com code-signing secrets in scope, so push triggers are now limited to main and release (manual staging builds remain via workflow_dispatch), and the secret-bearing jobs are gated behind the staging/production GitHub environments. Refs GH-049, GH-075 --- .github/workflows/device-agent-release.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/device-agent-release.yml b/.github/workflows/device-agent-release.yml index 2d63a20a38..90e703738d 100644 --- a/.github/workflows/device-agent-release.yml +++ b/.github/workflows/device-agent-release.yml @@ -1,9 +1,14 @@ name: Device Agent Release +# The push trigger is intentionally limited to protected branches (main, release). +# This pipeline handles Apple code-signing secrets (MAC_CSC_LINK, APPLE_ID, ...) and +# SSL.com eSigner secrets (ESIGNER_*) - broadening it to arbitrary branches would let +# anyone who can push a branch exfiltrate those credentials. Manual staging builds are +# still available via workflow_dispatch. on: workflow_dispatch: push: - branches: ['**'] + branches: ['main', 'release'] paths: - 'packages/device-agent/**' @@ -90,6 +95,7 @@ jobs: name: Build macOS (.dmg + .zip) needs: detect-version runs-on: macos-latest + environment: ${{ needs.detect-version.outputs.s3_env }} defaults: run: working-directory: packages/device-agent @@ -148,6 +154,7 @@ jobs: name: Build Windows (.exe) needs: detect-version runs-on: windows-latest + environment: ${{ needs.detect-version.outputs.s3_env }} defaults: run: working-directory: packages/device-agent @@ -418,6 +425,7 @@ jobs: name: Upload to S3 needs: [detect-version, build-macos, build-windows, build-linux] runs-on: ubuntu-latest + environment: ${{ needs.detect-version.outputs.s3_env }} steps: - name: Download all artifacts uses: actions/download-artifact@v4