mentor-mcp-server/examples/code-review.md

# 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