Skip to content

Commit a7ea199

Browse files
fix(ai): enforce audit ownership and normalize formatting
Add a ForbiddenException check so users can only chat about their own audits (fixes an IDOR), apply Prettier formatting, and resolve lint warnings to satisfy CI (--max-warnings 0).
1 parent e496e8d commit a7ea199

8 files changed

Lines changed: 152 additions & 176 deletions

File tree

apps/api/src/modules/ai/ai.controller.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,7 @@ export class AiController {
2323

2424
@Delete('conversation/:auditId')
2525
@ApiOperation({ summary: 'Clear conversation history for an audit' })
26-
clearConversation(
27-
@Param('auditId') auditId: string,
28-
@CurrentUser('id') userId: string,
29-
) {
26+
clearConversation(@Param('auditId') auditId: string, @CurrentUser('id') userId: string) {
3027
this.aiService.clearConversation(userId, auditId);
3128
return { success: true };
3229
}

apps/api/src/modules/ai/ai.module.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,13 @@ const aiEngineFactory = {
2121
if (!anthropicKey) {
2222
logger.warn('ANTHROPIC_API_KEY not set — AI chat will fail');
2323
}
24-
return new AiEngineService(
25-
new AnthropicProvider(anthropicKey ?? '', model),
26-
);
24+
return new AiEngineService(new AnthropicProvider(anthropicKey ?? '', model));
2725
}
2826

2927
if (!openAiKey) {
3028
logger.warn('OPENAI_API_KEY not set — AI chat will fail');
3129
}
32-
return new AiEngineService(
33-
new OpenAiProvider(openAiKey ?? '', model),
34-
);
30+
return new AiEngineService(new OpenAiProvider(openAiKey ?? '', model));
3531
},
3632
};
3733

apps/api/src/modules/ai/ai.service.spec.ts

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1-
import { InternalServerErrorException, NotFoundException } from '@nestjs/common';
1+
import {
2+
ForbiddenException,
3+
InternalServerErrorException,
4+
NotFoundException,
5+
} from '@nestjs/common';
26
import { Test, type TestingModule } from '@nestjs/testing';
3-
4-
import type { AiChatMessage } from '@veridion/shared';
57
import { AiService as AiEngineService } from '@veridion/ai-engine';
8+
import type { AiChatMessage } from '@veridion/shared';
69

710
import { PrismaService } from '../../common/prisma/prisma.service';
811
import { AiService } from './ai.service';
@@ -39,6 +42,7 @@ const mockAudit = {
3942
id: 'audit-1',
4043
status: 'COMPLETED',
4144
securityScore: 75,
45+
project: { userId: 'user-1' },
4246
findings: mockFindings,
4347
};
4448

@@ -90,9 +94,13 @@ describe('AiService', () => {
9094
it('should throw NotFoundException when audit does not exist', async () => {
9195
mockPrisma.db.audit.findUnique.mockResolvedValue(null);
9296

93-
await expect(service.chat('user-1', chatDto)).rejects.toThrow(
94-
NotFoundException,
95-
);
97+
await expect(service.chat('user-1', chatDto)).rejects.toThrow(NotFoundException);
98+
});
99+
100+
it('should throw ForbiddenException when the audit belongs to another user', async () => {
101+
mockPrisma.db.audit.findUnique.mockResolvedValue(mockAudit);
102+
103+
await expect(service.chat('user-2', chatDto)).rejects.toThrow(ForbiddenException);
96104
});
97105

98106
it('should return AI response with citations', async () => {
@@ -132,7 +140,7 @@ describe('AiService', () => {
132140
await service.chat('user-1', { ...chatDto, message: 'Tell me more' });
133141

134142
// The second call should include previous messages
135-
const secondCallMessages = mockAiEngine.chat.mock.calls[1]![0] as AiChatMessage[];
143+
const secondCallMessages = mockAiEngine.chat.mock.calls[1]?.[0] ?? [];
136144
const userMessages = secondCallMessages.filter((m) => m.role === 'user');
137145
expect(userMessages).toHaveLength(2);
138146
});
@@ -149,19 +157,17 @@ describe('AiService', () => {
149157
context: { findingId: 'finding-1' },
150158
});
151159

152-
const messages = mockAiEngine.chat.mock.calls[0]![0] as AiChatMessage[];
153-
const userMsg = messages.find((m) => m.role === 'user')!;
154-
expect(userMsg.content).toContain('Reentrancy Vulnerability');
155-
expect(userMsg.content).toContain('Reentrancy Vulnerability');
160+
const messages = mockAiEngine.chat.mock.calls[0]?.[0] ?? [];
161+
const userMsg = messages.find((m) => m.role === 'user');
162+
expect(userMsg?.content).toContain('Reentrancy Vulnerability');
163+
expect(userMsg?.content).toContain('Reentrancy Vulnerability');
156164
});
157165

158166
it('should throw InternalServerErrorException when AI engine fails', async () => {
159167
mockPrisma.db.audit.findUnique.mockResolvedValue(mockAudit);
160168
mockAiEngine.chat.mockRejectedValue(new Error('API rate limit exceeded'));
161169

162-
await expect(service.chat('user-1', chatDto)).rejects.toThrow(
163-
InternalServerErrorException,
164-
);
170+
await expect(service.chat('user-1', chatDto)).rejects.toThrow(InternalServerErrorException);
165171
});
166172

167173
it('should clear conversation', async () => {
@@ -177,7 +183,7 @@ describe('AiService', () => {
177183
// Next message should start fresh
178184
await service.chat('user-1', { ...chatDto, message: 'New question' });
179185

180-
const messages = mockAiEngine.chat.mock.calls[1]![0] as AiChatMessage[];
186+
const messages = mockAiEngine.chat.mock.calls[1]?.[0] ?? [];
181187
const userMessages = messages.filter((m) => m.role === 'user');
182188
expect(userMessages).toHaveLength(1);
183189
});

apps/api/src/modules/ai/ai.service.ts

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
1-
import { Injectable, InternalServerErrorException, NotFoundException } from '@nestjs/common';
2-
import type { AiChatMessage } from '@veridion/shared';
1+
import {
2+
ForbiddenException,
3+
Injectable,
4+
InternalServerErrorException,
5+
NotFoundException,
6+
} from '@nestjs/common';
37
import { AiService as AiEngineService } from '@veridion/ai-engine';
48
import { logger } from '@veridion/logger';
9+
import type { AiChatMessage } from '@veridion/shared';
510

611
import { PrismaService } from '../../common/prisma/prisma.service';
712
import type { AiChatDto } from './dto/ai.dto';
@@ -57,13 +62,21 @@ export class AiService {
5762
findings: {
5863
orderBy: { severity: 'asc' },
5964
},
65+
project: {
66+
select: { userId: true },
67+
},
6068
},
6169
});
6270

6371
if (!audit) {
6472
throw new NotFoundException(`Audit with ID ${dto.auditId} not found`);
6573
}
6674

75+
// Ensure the audit belongs to the authenticated user.
76+
if (audit.project.userId !== userId) {
77+
throw new ForbiddenException('Access denied');
78+
}
79+
6780
const conversationKey = `${userId}:${dto.auditId}`;
6881
const history = this.getOrCreateHistory(conversationKey);
6982

@@ -76,7 +89,8 @@ export class AiService {
7689
// Add the user message with optional context
7790
let userContent = dto.message;
7891
if (dto.context?.findingId) {
79-
const finding = audit.findings.find((f) => f.id === dto.context!.findingId);
92+
const { findingId } = dto.context;
93+
const finding = audit.findings.find((f) => f.id === findingId);
8094
if (finding) {
8195
userContent = `[Context: User is asking about finding "${finding.title}" (${finding.severity}) in ${finding.filePath}:${finding.lineStart}-${finding.lineEnd}]\n\n${dto.message}`;
8296
}
@@ -89,16 +103,12 @@ export class AiService {
89103
history.push({ role: 'user', content: userContent });
90104

91105
// Trim history if too long, keeping system message and recent messages
92-
const recentHistory = history.length > MAX_HISTORY_LENGTH
93-
? history.slice(-MAX_HISTORY_LENGTH)
94-
: history;
106+
const recentHistory =
107+
history.length > MAX_HISTORY_LENGTH ? history.slice(-MAX_HISTORY_LENGTH) : history;
95108

96109
const messages: AiChatMessage[] = [systemMessage, ...recentHistory];
97110

98-
logger.info(
99-
{ auditId: dto.auditId, userId, messageCount: messages.length },
100-
'AI chat request',
101-
);
111+
logger.info({ auditId: dto.auditId, userId, messageCount: messages.length }, 'AI chat request');
102112

103113
try {
104114
const response = await this.aiEngine.chat(messages);
@@ -121,10 +131,7 @@ export class AiService {
121131
citations,
122132
};
123133
} catch (error) {
124-
logger.error(
125-
{ error, auditId: dto.auditId, userId },
126-
'AI chat request failed',
127-
);
134+
logger.error({ error, auditId: dto.auditId, userId }, 'AI chat request failed');
128135
throw new InternalServerErrorException(
129136
'AI service is temporarily unavailable. Please try again later.',
130137
);
@@ -178,9 +185,10 @@ export class AiService {
178185
{} as Record<string, number>,
179186
);
180187

181-
const scoreLine = audit.securityScore !== null
182-
? `Security Score: ${audit.securityScore}/100`
183-
: 'Security Score: Not yet scored';
188+
const scoreLine =
189+
audit.securityScore !== null
190+
? `Security Score: ${audit.securityScore}/100`
191+
: 'Security Score: Not yet scored';
184192

185193
let prompt = `You are Veridion, an expert smart contract security assistant. You help developers understand and fix vulnerabilities in their smart contracts.
186194
@@ -239,7 +247,7 @@ ${finding.codeSnippet ? `- Code:\n\`\`\`solidity\n${finding.codeSnippet}\n\`\`\`
239247
let match: RegExpExecArray | null;
240248

241249
while ((match = citationRegex.exec(content)) !== null) {
242-
const ref = match[1]!.trim();
250+
const ref = (match[1] ?? '').trim();
243251

244252
// Try to match by UUID first
245253
const findingById = findingMap.get(ref);
@@ -255,9 +263,7 @@ ${finding.codeSnippet ? `- Code:\n\`\`\`solidity\n${finding.codeSnippet}\n\`\`\`
255263
}
256264

257265
// Try to match by title
258-
const findingByTitle = findings.find(
259-
(f) => f.title.toLowerCase() === ref.toLowerCase(),
260-
);
266+
const findingByTitle = findings.find((f) => f.title.toLowerCase() === ref.toLowerCase());
261267
if (findingByTitle) {
262268
if (!citations.some((c) => c.findingId === findingByTitle.id)) {
263269
citations.push({

0 commit comments

Comments
 (0)