Mentor MCP Server
by cyanheads
Verified
- mentor-mcp-server
- examples
# Code Review Tool Example
## Request
```json
{
"code_snippet": "import type { ToolDefinition, CodeReviewArgs, ToolResponse } from '../../types/index.js';\nimport { makeDeepseekAPICall, checkRateLimit } from '../../api/deepseek/deepseek.js';\nimport { readFileContent } from '../../utils/file.js';\nimport { createPrompt, PromptTemplate, sanitizeInput } from '../../utils/prompt.js';\n\nconst SYSTEM_PROMPT = `You are an expert code reviewer...`;\n\nconst PROMPT_TEMPLATE: PromptTemplate = {\n template: `Review the following {language} code...`,\n systemPrompt: SYSTEM_PROMPT\n};\n\nexport const definition: ToolDefinition = {\n name: 'code_review',\n description: 'Provides a code review...',\n inputSchema: {\n type: 'object',\n properties: {\n file_path: { type: 'string', description: 'The full path...' },\n language: { type: 'string', description: 'The programming language...' },\n code_snippet: { type: 'string', description: 'Optional small code...' }\n },\n oneOf: [\n { required: ['file_path', 'language'] },\n { required: ['code_snippet', 'language'] }\n ]\n }\n};\n\nexport async function handler(args: unknown): Promise<ToolResponse> {\n if (!checkRateLimit()) {\n return { content: [{ type: 'text', text: 'Rate limit exceeded' }], isError: true };\n }\n\n if (!args || typeof args !== 'object') {\n return { content: [{ type: 'text', text: 'Invalid arguments' }], isError: true };\n }\n\n if (!('language' in args) || typeof args.language !== 'string') {\n return { content: [{ type: 'text', text: 'Language required' }], isError: true };\n }\n\n try {\n let codeToReview: string;\n const typedArgs = args as CodeReviewArgs;\n\n if (typedArgs.file_path) {\n try {\n codeToReview = await readFileContent(typedArgs.file_path);\n } catch (error) {\n return {\n content: [{ type: 'text', text: `Error reading file: ${error}` }],\n isError: true\n };\n }\n } else if (typedArgs.code_snippet) {\n codeToReview = typedArgs.code_snippet;\n } else {\n return {\n content: [{ type: 'text', text: 'File path or snippet required' }],\n isError: true\n };\n }\n\n const sanitizedCode = sanitizeInput(codeToReview);\n const sanitizedLanguage = sanitizeInput(typedArgs.language);\n const prompt = createPrompt(PROMPT_TEMPLATE, {\n language: sanitizedLanguage,\n code: sanitizedCode\n });\n\n const response = await makeDeepseekAPICall(prompt, SYSTEM_PROMPT);\n\n if (response.isError) {\n return {\n content: [{ type: 'text', text: `Error: ${response.errorMessage}` }],\n isError: true\n };\n }\n\n return { content: [{ type: 'text', text: response.text }] };\n } catch (error) {\n console.error('Code review error:', error);\n return {\n content: [{ type: 'text', text: `Error: ${error}` }],\n isError: true\n };\n }\n}",
"language": "typescript"
}
```
## Response
```markdown
1. Critical Issues
------------------
- **Unsafe type assertion**:
`const typedArgs = args as CodeReviewArgs` bypasses proper type validation. An attacker could send malformed payloads that pass initial checks but fail later.
- **Incomplete input validation**:
While checking for `language`, the code doesn't validate types for `file_path` or `code_snippet`. An invalid numeric `file_path` would throw later.
- **Conflicting input handling**:
The schema specifies `oneOf` but code prioritizes `file_path` over `code_snippet` if both are present. This contradicts the schema definition.
2. Security Concerns
--------------------
- **Error message leakage**:
Returning raw error messages (e.g., `Error reading file: ${error}`) could expose sensitive filesystem information.
- **Potential prompt injection**:
While `sanitizeInput` is used, its implementation isn't shown. Inadequate sanitization could allow LLM prompt injections via user-provided code snippets.
- **Early rate limiting**:
`checkRateLimit()` is called before input validation, allowing invalid requests to consume rate quota.
3. Performance Considerations
------------------------------
- **Duplicate sanitization**:
Both `codeToReview` and `language` are sanitized, but this might be redundant if the API layer already handles sanitization.
- **Unbounded file reading**:
`readFileContent` could cause memory issues with large files. Consider adding size limits.
4. Code Style & Best Practices
-------------------------------
- **Type safety violations**:
- Using type assertion (`as`) instead of proper type guards
- No Zod-like validation for input schema
- `unknown` type in handler parameters not properly narrowed
- **Error handling inconsistencies**:
- Mix of error return objects and exceptions
- `console.error` in production code (should use structured logging)
- **Template issues**:
- Hardcoded SYSTEM_PROMPT makes testing/mocking difficult
- Prompt construction could be vulnerable to injection if sanitizeInput isn't robust
5. Suggested Improvements
--------------------------
**Input Validation**:
```typescript
// Use type guard instead of assertion
function isCodeReviewArgs(args: unknown): args is CodeReviewArgs {
return /* detailed property checks */;
}
// Validate file_path format if applicable
if (typedArgs.file_path && !isValidPath(typedArgs.file_path)) {
return invalidPathError;
}
```
**Security Enhancements**:
- Implement generic error messages:
```typescript
return {
content: [{ type: 'text', text: 'File read error' }],
isError: true
};
```
- Add file size limits:
```typescript
const MAX_FILE_SIZE = 1024 * 1024; // 1MB
if (fileStats.size > MAX_FILE_SIZE) throw new Error('File too large');
```
**Performance Improvements**:
- Move rate limit check after input validation:
```typescript
export async function handler(args: unknown): Promise<ToolResponse> {
// Validate FIRST
if (!validateArgs(args)) return invalidArgsResponse;
// Then check rate limits
if (!checkRateLimit()) return rateLimitResponse;
```
**TypeScript Best Practices**:
1. Use Zod for schema validation:
```typescript
import { z } from 'zod';
const CodeReviewSchema = z.union([
z.object({
file_path: z.string().min(1),
language: z.string().min(1)
}),
z.object({
code_snippet: z.string().min(1),
language: z.string().min(1)
})
]);
```
2. Implement proper error typing:
```typescript
class CodeReviewError extends Error {
public readonly isUserSafe: boolean;
constructor(message: string, isUserSafe: boolean) {
super(message);
this.isUserSafe = isUserSafe;
}
}
```
**Architecture Suggestions**:
- Separate validation, business logic, and presentation layers
- Use dependency injection for file system access and API calls
- Implement circuit breakers for the Deepseek API integration
- Add telemetry for code review request metrics
**Additional Considerations**:
- Add timeout handling for file operations and API calls
- Implement content scanning for malicious code patterns in snippets
- Add cache control headers for API responses
- Consider streaming partial responses for large reviews
- Add content security policies for any HTML output