diff --git a/.claude/skills/code-reviewer/README.md b/.claude/skills/code-reviewer/README.md new file mode 100644 index 00000000..27810044 --- /dev/null +++ b/.claude/skills/code-reviewer/README.md @@ -0,0 +1,340 @@ +# Code Reviewer Skill + +A comprehensive code review skill for identifying security vulnerabilities and best practices violations. + +## Overview + +The **code-reviewer** skill provides automated code review capabilities covering: +- **Security Analysis**: OWASP Top 10, CWE Top 25, language-specific vulnerabilities +- **Code Quality**: Naming conventions, complexity, duplication, dead code +- **Performance**: N+1 queries, inefficient algorithms, memory leaks +- **Maintainability**: Documentation, test coverage, dependency health + +## Quick Start + +### Basic Usage + +```bash +# Review entire codebase +/code-reviewer + +# Review specific directory +/code-reviewer --scope src/auth + +# Focus on security only +/code-reviewer --focus security + +# Focus on best practices only +/code-reviewer --focus best-practices +``` + +### Advanced Options + +```bash +# Review with custom severity threshold +/code-reviewer --severity critical,high + +# Review specific file types +/code-reviewer --languages typescript,python + +# Generate detailed report +/code-reviewer --report-level detailed + +# Resume from previous session +/code-reviewer --resume +``` + +## Features + +### Security Analysis + +✅ **OWASP Top 10 2021 Coverage** +- Injection vulnerabilities (SQL, Command, XSS) +- Authentication & authorization flaws +- Sensitive data exposure +- Security misconfiguration +- And more... + +✅ **CWE Top 25 Coverage** +- Cross-site scripting (CWE-79) +- SQL injection (CWE-89) +- Command injection (CWE-78) +- Input validation (CWE-20) +- And more... + +✅ **Language-Specific Checks** +- JavaScript/TypeScript: prototype pollution, eval usage +- Python: pickle vulnerabilities, command injection +- Java: deserialization, XXE +- Go: race conditions, memory leaks + +### Best Practices Review + +✅ **Code Quality** +- Naming convention compliance +- Cyclomatic complexity analysis +- Code duplication detection +- Dead code identification + +✅ **Performance** +- N+1 query detection +- Inefficient algorithm patterns +- Memory leak detection +- Resource cleanup verification + +✅ **Maintainability** +- Documentation coverage +- Test coverage analysis +- Dependency health check +- Error handling review + +## Output + +The skill generates comprehensive reports in `.code-review/` directory: + +``` +.code-review/ +├── inventory.json # File inventory with metadata +├── security-findings.json # Security vulnerabilities +├── best-practices-findings.json # Best practices violations +├── summary.json # Summary statistics +├── REPORT.md # Comprehensive markdown report +└── FIX-CHECKLIST.md # Actionable fix checklist +``` + +### Report Contents + +**REPORT.md** includes: +- Executive summary with risk assessment +- Quality scores (Security, Code Quality, Performance, Maintainability) +- Detailed findings organized by severity +- Code examples with fix recommendations +- Action plan prioritized by urgency +- Compliance status (PCI DSS, HIPAA, GDPR, SOC 2) + +**FIX-CHECKLIST.md** provides: +- Checklist format for tracking fixes +- Organized by severity (Critical → Low) +- Effort estimates for each issue +- Priority assignments + +## Configuration + +Create `.code-reviewer.json` in project root: + +```json +{ + "scope": { + "include": ["src/**/*", "lib/**/*"], + "exclude": ["**/*.test.ts", "**/*.spec.ts", "**/node_modules/**"] + }, + "security": { + "enabled": true, + "checks": ["owasp-top-10", "cwe-top-25"], + "severity_threshold": "medium" + }, + "best_practices": { + "enabled": true, + "code_quality": true, + "performance": true, + "maintainability": true + }, + "reporting": { + "format": "markdown", + "output_path": ".code-review/", + "include_snippets": true, + "include_fixes": true + } +} +``` + +## Workflow + +### Phase 1: Code Discovery +- Discover and categorize code files +- Extract metadata (LOC, complexity, framework) +- Prioritize files (Critical, High, Medium, Low) + +### Phase 2: Security Analysis +- Scan for OWASP Top 10 vulnerabilities +- Check CWE Top 25 weaknesses +- Apply language-specific security patterns +- Generate security findings + +### Phase 3: Best Practices Review +- Analyze code quality issues +- Detect performance problems +- Assess maintainability concerns +- Generate best practices findings + +### Phase 4: Report Generation +- Consolidate all findings +- Calculate quality scores +- Generate comprehensive reports +- Create actionable checklists + +## Integration + +### Pre-commit Hook + +Block commits with critical/high issues: + +```bash +#!/bin/bash +# .git/hooks/pre-commit + +staged_files=$(git diff --cached --name-only --diff-filter=ACMR) +ccw run code-reviewer --scope "$staged_files" --severity critical,high + +if [ $? -ne 0 ]; then + echo "❌ Code review found critical/high issues. Commit aborted." + exit 1 +fi +``` + +### CI/CD Integration + +```yaml +# .github/workflows/code-review.yml +name: Code Review +on: [pull_request] + +jobs: + review: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Run Code Review + run: | + ccw run code-reviewer --report-level detailed + ccw report upload .code-review/report.md +``` + +## Examples + +### Example 1: Security-Focused Review + +```bash +# Review authentication module for security issues +/code-reviewer --scope src/auth --focus security --severity critical,high +``` + +**Output**: Security findings with OWASP/CWE mappings and fix recommendations + +### Example 2: Performance Review + +```bash +# Review API endpoints for performance issues +/code-reviewer --scope src/api --focus best-practices --check performance +``` + +**Output**: N+1 queries, inefficient algorithms, memory leak detections + +### Example 3: Full Project Audit + +```bash +# Comprehensive review of entire codebase +/code-reviewer --report-level detailed --output .code-review/audit-2024-01.md +``` + +**Output**: Complete audit with all findings, scores, and action plan + +## Compliance Support + +The skill maps findings to compliance requirements: + +- **PCI DSS**: Requirement 6.5 (Common coding vulnerabilities) +- **HIPAA**: Technical safeguards and access controls +- **GDPR**: Article 32 (Security of processing) +- **SOC 2**: Security controls and monitoring + +## Architecture + +### Execution Mode +**Sequential** - Fixed phase order for systematic review: +1. Code Discovery → 2. Security Analysis → 3. Best Practices → 4. Report Generation + +### Tools Used +- `mcp__ace-tool__search_context` - Semantic code search +- `mcp__ccw-tools__smart_search` - Pattern matching +- `Read` - File content access +- `Write` - Report generation + +## Quality Standards + +### Scoring System + +``` +Overall Score = ( + Security Score × 0.4 + + Code Quality Score × 0.25 + + Performance Score × 0.2 + + Maintainability Score × 0.15 +) +``` + +### Score Ranges +- **A (90-100)**: Excellent - Production ready +- **B (80-89)**: Good - Minor improvements needed +- **C (70-79)**: Acceptable - Some issues to address +- **D (60-69)**: Poor - Significant improvements required +- **F (0-59)**: Failing - Major issues, not production ready + +## Troubleshooting + +### Large Codebase + +If review takes too long: +```bash +# Review in batches +/code-reviewer --scope src/module-1 +/code-reviewer --scope src/module-2 --resume + +# Or use parallel execution +/code-reviewer --parallel 4 +``` + +### False Positives + +Configure suppressions in `.code-reviewer.json`: +```json +{ + "suppressions": { + "security": { + "sql-injection": { + "paths": ["src/legacy/**/*"], + "reason": "Legacy code, scheduled for refactor" + } + } + } +} +``` + +## File Structure + +``` +.claude/skills/code-reviewer/ +├── SKILL.md # Main skill documentation +├── README.md # This file +├── phases/ +│ ├── 01-code-discovery.md +│ ├── 02-security-analysis.md +│ ├── 03-best-practices-review.md +│ └── 04-report-generation.md +├── specs/ +│ ├── security-requirements.md +│ ├── best-practices-requirements.md +│ └── quality-standards.md +└── templates/ + ├── security-finding.md + ├── best-practice-finding.md + └── report-template.md +``` + +## Version + +**v1.0.0** - Initial release + +## License + +MIT License diff --git a/.claude/skills/code-reviewer/SKILL.md b/.claude/skills/code-reviewer/SKILL.md new file mode 100644 index 00000000..6051b741 --- /dev/null +++ b/.claude/skills/code-reviewer/SKILL.md @@ -0,0 +1,317 @@ +# Code Reviewer + +Comprehensive code review skill for identifying security vulnerabilities and best practices violations. + +## Metadata + +```yaml +name: code-reviewer +description: 帮助审查代码的安全漏洞和最佳实践 +version: 1.0.0 +execution_mode: sequential +allowed-tools: + - Read + - Glob + - Grep + - mcp__ace-tool__search_context + - mcp__ccw-tools__smart_search +``` + +## Architecture Overview + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ Code Reviewer Workflow │ +├─────────────────────────────────────────────────────────────────┤ +│ │ +│ Phase 1: Code Discovery → 发现待审查的代码文件 │ +│ & Scoping - 根据语言/框架识别文件 │ +│ ↓ - 设置审查范围和优先级 │ +│ │ +│ Phase 2: Security → 安全漏洞扫描 │ +│ Analysis - OWASP Top 10 检查 │ +│ ↓ - 常见漏洞模式识别 │ +│ - 敏感数据泄露检查 │ +│ │ +│ Phase 3: Best Practices → 最佳实践审查 │ +│ Review - 代码质量检查 │ +│ ↓ - 性能优化建议 │ +│ - 可维护性评估 │ +│ │ +│ Phase 4: Report → 生成审查报告 │ +│ Generation - 按严重程度分类问题 │ +│ - 提供修复建议和示例 │ +│ - 生成可追踪的修复清单 │ +│ │ +└─────────────────────────────────────────────────────────────────┘ +``` + +## Features + +### Security Analysis + +- **OWASP Top 10 Coverage** + - Injection vulnerabilities (SQL, Command, LDAP) + - Authentication & authorization bypass + - Sensitive data exposure + - XML External Entities (XXE) + - Broken access control + - Security misconfiguration + - Cross-Site Scripting (XSS) + - Insecure deserialization + - Components with known vulnerabilities + - Insufficient logging & monitoring + +- **Language-Specific Checks** + - JavaScript/TypeScript: prototype pollution, eval usage + - Python: pickle vulnerabilities, command injection + - Java: deserialization, path traversal + - Go: race conditions, memory leaks + +### Best Practices Review + +- **Code Quality** + - Naming conventions + - Function complexity (cyclomatic complexity) + - Code duplication + - Dead code detection + +- **Performance** + - N+1 queries + - Inefficient algorithms + - Memory leaks + - Resource cleanup + +- **Maintainability** + - Documentation quality + - Test coverage + - Error handling patterns + - Dependency management + +## Usage + +### Basic Review + +```bash +# Review entire codebase +/code-reviewer + +# Review specific directory +/code-reviewer --scope src/auth + +# Focus on security only +/code-reviewer --focus security + +# Focus on best practices only +/code-reviewer --focus best-practices +``` + +### Advanced Options + +```bash +# Review with custom severity threshold +/code-reviewer --severity critical,high + +# Review specific file types +/code-reviewer --languages typescript,python + +# Generate detailed report with code snippets +/code-reviewer --report-level detailed + +# Resume from previous session +/code-reviewer --resume +``` + +## Configuration + +Create `.code-reviewer.json` in project root: + +```json +{ + "scope": { + "include": ["src/**/*", "lib/**/*"], + "exclude": ["**/*.test.ts", "**/*.spec.ts", "**/node_modules/**"] + }, + "security": { + "enabled": true, + "checks": ["owasp-top-10", "cwe-top-25"], + "severity_threshold": "medium" + }, + "best_practices": { + "enabled": true, + "code_quality": true, + "performance": true, + "maintainability": true + }, + "reporting": { + "format": "markdown", + "output_path": ".code-review/", + "include_snippets": true, + "include_fixes": true + } +} +``` + +## Output + +### Review Report Structure + +```markdown +# Code Review Report + +## Executive Summary +- Total Issues: 42 +- Critical: 3 +- High: 8 +- Medium: 15 +- Low: 16 + +## Security Findings + +### [CRITICAL] SQL Injection in User Query +**File**: src/auth/user-service.ts:145 +**Issue**: Unsanitized user input in SQL query +**Fix**: Use parameterized queries + +Code Snippet: +\`\`\`typescript +// ❌ Vulnerable +const query = `SELECT * FROM users WHERE username = '${username}'`; + +// ✅ Fixed +const query = 'SELECT * FROM users WHERE username = ?'; +db.execute(query, [username]); +\`\`\` + +## Best Practices Findings + +### [MEDIUM] High Cyclomatic Complexity +**File**: src/utils/validator.ts:78 +**Issue**: Function has complexity score of 15 (threshold: 10) +**Fix**: Break into smaller functions + +... +``` + +## Phase Documentation + +| Phase | Description | Output | +|-------|-------------|--------| +| [01-code-discovery.md](phases/01-code-discovery.md) | Discover and categorize code files | File inventory with metadata | +| [02-security-analysis.md](phases/02-security-analysis.md) | Analyze security vulnerabilities | Security findings list | +| [03-best-practices-review.md](phases/03-best-practices-review.md) | Review code quality and practices | Best practices findings | +| [04-report-generation.md](phases/04-report-generation.md) | Generate comprehensive report | Markdown report | + +## Specifications + +- [specs/security-requirements.md](specs/security-requirements.md) - Security check specifications +- [specs/best-practices-requirements.md](specs/best-practices-requirements.md) - Best practices standards +- [specs/quality-standards.md](specs/quality-standards.md) - Overall quality standards +- [specs/severity-classification.md](specs/severity-classification.md) - Issue severity criteria + +## Templates + +- [templates/security-finding.md](templates/security-finding.md) - Security finding template +- [templates/best-practice-finding.md](templates/best-practice-finding.md) - Best practice finding template +- [templates/report-template.md](templates/report-template.md) - Final report template + +## Integration with Development Workflow + +### Pre-commit Hook + +```bash +#!/bin/bash +# .git/hooks/pre-commit + +# Run code review on staged files +staged_files=$(git diff --cached --name-only --diff-filter=ACMR) +ccw run code-reviewer --scope "$staged_files" --severity critical,high + +if [ $? -ne 0 ]; then + echo "❌ Code review found critical/high issues. Commit aborted." + exit 1 +fi +``` + +### CI/CD Integration + +```yaml +# .github/workflows/code-review.yml +name: Code Review +on: [pull_request] + +jobs: + review: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Run Code Review + run: | + ccw run code-reviewer --report-level detailed + ccw report upload .code-review/report.md +``` + +## Examples + +### Example 1: Security-Focused Review + +```bash +# Review authentication module for security issues +/code-reviewer --scope src/auth --focus security --severity critical,high +``` + +### Example 2: Performance Review + +```bash +# Review API endpoints for performance issues +/code-reviewer --scope src/api --focus best-practices --check performance +``` + +### Example 3: Full Project Audit + +```bash +# Comprehensive review of entire codebase +/code-reviewer --report-level detailed --output .code-review/audit-2024-01.md +``` + +## Troubleshooting + +### Large Codebase + +If review takes too long: +```bash +# Review in batches +/code-reviewer --scope src/module-1 +/code-reviewer --scope src/module-2 --resume + +# Or use parallel execution +/code-reviewer --parallel 4 +``` + +### False Positives + +Configure suppressions in `.code-reviewer.json`: +```json +{ + "suppressions": { + "security": { + "sql-injection": { + "paths": ["src/legacy/**/*"], + "reason": "Legacy code, scheduled for refactor" + } + } + } +} +``` + +## Roadmap + +- [ ] AI-powered vulnerability detection +- [ ] Integration with popular security scanners (Snyk, SonarQube) +- [ ] Automated fix suggestions with diffs +- [ ] IDE plugins for real-time feedback +- [ ] Custom rule engine for organization-specific policies + +## License + +MIT License - See LICENSE file for details diff --git a/.claude/skills/code-reviewer/phases/01-code-discovery.md b/.claude/skills/code-reviewer/phases/01-code-discovery.md new file mode 100644 index 00000000..eab727f0 --- /dev/null +++ b/.claude/skills/code-reviewer/phases/01-code-discovery.md @@ -0,0 +1,246 @@ +# Phase 1: Code Discovery & Scoping + +## Objective + +Discover and categorize all code files within the specified scope, preparing them for security analysis and best practices review. + +## Input + +- **User Arguments**: + - `--scope`: Directory or file patterns (default: entire project) + - `--languages`: Specific languages to review (e.g., typescript, python, java) + - `--exclude`: Patterns to exclude (e.g., test files, node_modules) + +- **Configuration**: `.code-reviewer.json` (if exists) + +## Process + +### Step 1: Load Configuration + +```javascript +// Check for project-level configuration +const configPath = path.join(projectRoot, '.code-reviewer.json'); +const config = fileExists(configPath) + ? JSON.parse(readFile(configPath)) + : getDefaultConfig(); + +// Merge user arguments with config +const scope = args.scope || config.scope.include; +const exclude = args.exclude || config.scope.exclude; +const languages = args.languages || config.languages || 'auto'; +``` + +### Step 2: Discover Files + +Use MCP tools for efficient file discovery: + +```javascript +// Use smart_search for file discovery +const files = await mcp__ccw_tools__smart_search({ + action: "find_files", + pattern: scope, + includeHidden: false +}); + +// Apply exclusion patterns +const filteredFiles = files.filter(file => { + return !exclude.some(pattern => minimatch(file, pattern)); +}); +``` + +### Step 3: Categorize Files + +Categorize files by: +- **Language/Framework**: TypeScript, Python, Java, Go, etc. +- **File Type**: Source, config, test, build +- **Priority**: Critical (auth, payment), High (API), Medium (utils), Low (docs) + +```javascript +const inventory = { + critical: { + auth: ['src/auth/login.ts', 'src/auth/jwt.ts'], + payment: ['src/payment/stripe.ts'], + }, + high: { + api: ['src/api/users.ts', 'src/api/orders.ts'], + database: ['src/db/queries.ts'], + }, + medium: { + utils: ['src/utils/validator.ts'], + services: ['src/services/*.ts'], + }, + low: { + types: ['src/types/*.ts'], + } +}; +``` + +### Step 4: Extract Metadata + +For each file, extract: +- **Lines of Code (LOC)** +- **Complexity Indicators**: Function count, class count +- **Dependencies**: Import statements +- **Framework Detection**: Express, React, Django, etc. + +```javascript +const metadata = files.map(file => ({ + path: file, + language: detectLanguage(file), + loc: countLines(file), + complexity: estimateComplexity(file), + framework: detectFramework(file), + priority: categorizePriority(file) +})); +``` + +## Output + +### File Inventory + +Save to `.code-review/inventory.json`: + +```json +{ + "scan_date": "2024-01-15T10:30:00Z", + "total_files": 247, + "by_language": { + "typescript": 185, + "python": 42, + "javascript": 15, + "go": 5 + }, + "by_priority": { + "critical": 12, + "high": 45, + "medium": 120, + "low": 70 + }, + "files": [ + { + "path": "src/auth/login.ts", + "language": "typescript", + "loc": 245, + "functions": 8, + "classes": 2, + "priority": "critical", + "framework": "express", + "dependencies": ["bcrypt", "jsonwebtoken", "express"] + } + ] +} +``` + +### Summary Report + +```markdown +## Code Discovery Summary + +**Scope**: src/**/* +**Total Files**: 247 +**Languages**: TypeScript (75%), Python (17%), JavaScript (6%), Go (2%) + +### Priority Distribution +- Critical: 12 files (authentication, payment processing) +- High: 45 files (API endpoints, database queries) +- Medium: 120 files (utilities, services) +- Low: 70 files (types, configs) + +### Key Areas Identified +1. **Authentication Module** (src/auth/) - 12 files, 2,400 LOC +2. **Payment Processing** (src/payment/) - 5 files, 1,200 LOC +3. **API Layer** (src/api/) - 35 files, 5,600 LOC +4. **Database Layer** (src/db/) - 8 files, 1,800 LOC + +**Next Phase**: Security Analysis on Critical + High priority files +``` + +## State Management + +Save phase state for potential resume: + +```json +{ + "phase": "01-code-discovery", + "status": "completed", + "timestamp": "2024-01-15T10:35:00Z", + "output": { + "inventory_path": ".code-review/inventory.json", + "total_files": 247, + "critical_files": 12, + "high_files": 45 + } +} +``` + +## Agent Instructions + +```markdown +You are in Phase 1 of the Code Review workflow. Your task is to discover and categorize code files. + +**Instructions**: +1. Use mcp__ccw_tools__smart_search with action="find_files" to discover files +2. Apply exclusion patterns from config or arguments +3. Categorize files by language, type, and priority +4. Extract basic metadata (LOC, complexity indicators) +5. Save inventory to .code-review/inventory.json +6. Generate summary report +7. Proceed to Phase 2 with critical + high priority files + +**Tools Available**: +- mcp__ccw_tools__smart_search (file discovery) +- Read (read configuration and sample files) +- Write (save inventory and reports) + +**Output Requirements**: +- inventory.json with complete file list and metadata +- Summary markdown report +- State file for phase tracking +``` + +## Error Handling + +### No Files Found + +```javascript +if (filteredFiles.length === 0) { + throw new Error(`No files found matching scope: ${scope} + + Suggestions: + - Check if scope pattern is correct + - Verify exclude patterns are not too broad + - Ensure project has code files in specified scope + `); +} +``` + +### Large Codebase + +```javascript +if (filteredFiles.length > 1000) { + console.warn(`⚠️ Large codebase detected (${filteredFiles.length} files)`); + console.log(`Consider using --scope to review in batches`); + + // Offer to focus on critical/high priority only + const answer = await askUser("Review critical/high priority files only?"); + if (answer === 'yes') { + filteredFiles = filteredFiles.filter(f => + f.priority === 'critical' || f.priority === 'high' + ); + } +} +``` + +## Validation + +Before proceeding to Phase 2: + +- ✅ Inventory file created +- ✅ At least one file categorized as critical or high priority +- ✅ Metadata extracted for all files +- ✅ Summary report generated +- ✅ State saved for resume capability + +## Next Phase + +**Phase 2: Security Analysis** - Analyze critical and high priority files for security vulnerabilities using OWASP Top 10 and CWE Top 25 checks. diff --git a/.claude/skills/code-reviewer/phases/02-security-analysis.md b/.claude/skills/code-reviewer/phases/02-security-analysis.md new file mode 100644 index 00000000..9fc1a244 --- /dev/null +++ b/.claude/skills/code-reviewer/phases/02-security-analysis.md @@ -0,0 +1,442 @@ +# Phase 2: Security Analysis + +## Objective + +Analyze code files for security vulnerabilities based on OWASP Top 10, CWE Top 25, and language-specific security patterns. + +## Input + +- **File Inventory**: From Phase 1 (`.code-review/inventory.json`) +- **Priority Focus**: Critical and High priority files (unless `--scope all`) +- **User Arguments**: + - `--focus security`: Security-only mode + - `--severity critical,high,medium,low`: Minimum severity to report + - `--checks`: Specific security checks to run (e.g., sql-injection, xss) + +## Process + +### Step 1: Load Security Rules + +```javascript +// Load security check definitions +const securityRules = { + owasp_top_10: [ + 'injection', + 'broken_authentication', + 'sensitive_data_exposure', + 'xxe', + 'broken_access_control', + 'security_misconfiguration', + 'xss', + 'insecure_deserialization', + 'vulnerable_components', + 'insufficient_logging' + ], + cwe_top_25: [ + 'cwe-79', // XSS + 'cwe-89', // SQL Injection + 'cwe-20', // Improper Input Validation + 'cwe-78', // OS Command Injection + 'cwe-190', // Integer Overflow + // ... more CWE checks + ] +}; + +// Load language-specific rules +const languageRules = { + typescript: require('./rules/typescript-security.json'), + python: require('./rules/python-security.json'), + java: require('./rules/java-security.json'), + go: require('./rules/go-security.json'), +}; +``` + +### Step 2: Analyze Files for Vulnerabilities + +For each file in the inventory, perform security analysis: + +```javascript +const findings = []; + +for (const file of inventory.files) { + if (file.priority !== 'critical' && file.priority !== 'high') continue; + + // Read file content + const content = await Read({ file_path: file.path }); + + // Run security checks + const fileFindings = await runSecurityChecks(content, file, { + rules: securityRules, + languageRules: languageRules[file.language], + severity: args.severity || 'medium' + }); + + findings.push(...fileFindings); +} +``` + +### Step 3: Security Check Patterns + +#### A. Injection Vulnerabilities + +**SQL Injection**: +```javascript +// Pattern: String concatenation in SQL queries +const sqlInjectionPatterns = [ + /\$\{.*\}.*SELECT/, // Template literal with SELECT + /"SELECT.*\+\s*\w+/, // String concatenation + /execute\([`'"].*\$\{.*\}.*[`'"]\)/, // Parameterized query bypass + /query\(.*\+.*\)/, // Query concatenation +]; + +// Check code +for (const pattern of sqlInjectionPatterns) { + const matches = content.matchAll(new RegExp(pattern, 'g')); + for (const match of matches) { + findings.push({ + type: 'sql-injection', + severity: 'critical', + line: getLineNumber(content, match.index), + code: match[0], + file: file.path, + message: 'Potential SQL injection vulnerability', + recommendation: 'Use parameterized queries or ORM methods', + cwe: 'CWE-89', + owasp: 'A03:2021 - Injection' + }); + } +} +``` + +**Command Injection**: +```javascript +// Pattern: Unsanitized input in exec/spawn +const commandInjectionPatterns = [ + /exec\(.*\$\{.*\}/, // exec with template literal + /spawn\(.*,\s*\[.*\$\{.*\}.*\]\)/, // spawn with unsanitized args + /execSync\(.*\+.*\)/, // execSync with concatenation +]; +``` + +**XSS (Cross-Site Scripting)**: +```javascript +// Pattern: Unsanitized user input in DOM/HTML +const xssPatterns = [ + /innerHTML\s*=.*\$\{.*\}/, // innerHTML with template literal + /dangerouslySetInnerHTML/, // React dangerous prop + /document\.write\(.*\)/, // document.write + /<\w+.*\$\{.*\}.*>/, // JSX with unsanitized data +]; +``` + +#### B. Authentication & Authorization + +```javascript +// Pattern: Weak authentication +const authPatterns = [ + /password\s*===?\s*['"]/, // Hardcoded password comparison + /jwt\.sign\(.*,\s*['"][^'"]{1,16}['"]\)/, // Weak JWT secret + /bcrypt\.hash\(.*,\s*[1-9]\s*\)/, // Low bcrypt rounds + /md5\(.*password.*\)/, // MD5 for passwords + /if\s*\(\s*user\s*\)\s*\{/, // Missing auth check +]; + +// Check for missing authorization +const authzPatterns = [ + /router\.(get|post|put|delete)\(.*\)\s*=>/, // No middleware + /app\.use\([^)]*\)\s*;(?!.*auth)/, // Missing auth middleware +]; +``` + +#### C. Sensitive Data Exposure + +```javascript +// Pattern: Sensitive data in logs/responses +const sensitiveDataPatterns = [ + /(password|secret|token|key)\s*:/i, // Sensitive keys in objects + /console\.log\(.*password.*\)/i, // Password in logs + /res\.send\(.*user.*password.*\)/, // Password in response + /(api_key|apikey)\s*=\s*['"]/i, // Hardcoded API keys +]; +``` + +#### D. Security Misconfiguration + +```javascript +// Pattern: Insecure configurations +const misconfigPatterns = [ + /cors\(\{.*origin:\s*['"]?\*['"]?.*\}\)/, // CORS wildcard + /https?\s*:\s*false/, // HTTPS disabled + /helmet\(\)/, // Missing helmet config + /strictMode\s*:\s*false/, // Strict mode disabled +]; +``` + +### Step 4: Language-Specific Checks + +**TypeScript/JavaScript**: +```javascript +const jsFindings = [ + checkPrototypePollution(content), + checkEvalUsage(content), + checkUnsafeRegex(content), + checkWeakCrypto(content), +]; +``` + +**Python**: +```javascript +const pythonFindings = [ + checkPickleVulnerabilities(content), + checkYamlUnsafeLoad(content), + checkSqlAlchemy(content), + checkFlaskSecurityHeaders(content), +]; +``` + +**Java**: +```javascript +const javaFindings = [ + checkDeserialization(content), + checkXXE(content), + checkPathTraversal(content), + checkSQLInjection(content), +]; +``` + +**Go**: +```javascript +const goFindings = [ + checkRaceConditions(content), + checkSQLInjection(content), + checkPathTraversal(content), + checkCryptoWeakness(content), +]; +``` + +## Output + +### Security Findings File + +Save to `.code-review/security-findings.json`: + +```json +{ + "scan_date": "2024-01-15T11:00:00Z", + "total_findings": 24, + "by_severity": { + "critical": 3, + "high": 8, + "medium": 10, + "low": 3 + }, + "by_category": { + "injection": 5, + "authentication": 3, + "data_exposure": 4, + "misconfiguration": 6, + "xss": 3, + "other": 3 + }, + "findings": [ + { + "id": "SEC-001", + "type": "sql-injection", + "severity": "critical", + "file": "src/auth/user-service.ts", + "line": 145, + "column": 12, + "code": "const query = `SELECT * FROM users WHERE username = '${username}'`;", + "message": "SQL Injection vulnerability: User input directly concatenated in SQL query", + "cwe": "CWE-89", + "owasp": "A03:2021 - Injection", + "recommendation": { + "description": "Use parameterized queries to prevent SQL injection", + "fix_example": "const query = 'SELECT * FROM users WHERE username = ?';\ndb.execute(query, [username]);" + }, + "references": [ + "https://owasp.org/www-community/attacks/SQL_Injection", + "https://cwe.mitre.org/data/definitions/89.html" + ] + } + ] +} +``` + +### Security Report + +Generate markdown report: + +```markdown +# Security Analysis Report + +**Scan Date**: 2024-01-15 11:00:00 +**Files Analyzed**: 57 (Critical + High priority) +**Total Findings**: 24 + +## Severity Summary + +| Severity | Count | Percentage | +|----------|-------|------------| +| Critical | 3 | 12.5% | +| High | 8 | 33.3% | +| Medium | 10 | 41.7% | +| Low | 3 | 12.5% | + +## Critical Findings (Requires Immediate Action) + +### 🔴 [SEC-001] SQL Injection in User Authentication + +**File**: `src/auth/user-service.ts:145` +**CWE**: CWE-89 | **OWASP**: A03:2021 - Injection + +**Vulnerable Code**: +\`\`\`typescript +const query = \`SELECT * FROM users WHERE username = '\${username}'\`; +const user = await db.execute(query); +\`\`\` + +**Issue**: User input (`username`) is directly concatenated into SQL query, allowing attackers to inject malicious SQL commands. + +**Attack Example**: +\`\`\` +username: ' OR '1'='1' -- +Result: SELECT * FROM users WHERE username = '' OR '1'='1' --' +Effect: Bypasses authentication, returns all users +\`\`\` + +**Recommended Fix**: +\`\`\`typescript +// Use parameterized queries +const query = 'SELECT * FROM users WHERE username = ?'; +const user = await db.execute(query, [username]); + +// Or use ORM +const user = await User.findOne({ where: { username } }); +\`\`\` + +**References**: +- [OWASP SQL Injection](https://owasp.org/www-community/attacks/SQL_Injection) +- [CWE-89](https://cwe.mitre.org/data/definitions/89.html) + +--- + +### 🔴 [SEC-002] Hardcoded JWT Secret + +**File**: `src/auth/jwt.ts:23` +**CWE**: CWE-798 | **OWASP**: A07:2021 - Identification and Authentication Failures + +**Vulnerable Code**: +\`\`\`typescript +const token = jwt.sign(payload, 'mysecret123', { expiresIn: '1h' }); +\`\`\` + +**Issue**: JWT secret is hardcoded and weak (only 11 characters). + +**Recommended Fix**: +\`\`\`typescript +// Use environment variable with strong secret +const token = jwt.sign(payload, process.env.JWT_SECRET, { + expiresIn: '1h', + algorithm: 'HS256' +}); + +// Generate strong secret (32+ bytes): +// node -e "console.log(require('crypto').randomBytes(32).toString('hex'))" +\`\`\` + +--- + +## High Findings + +### 🟠 [SEC-003] Missing Input Validation + +**File**: `src/api/users.ts:67` +**CWE**: CWE-20 | **OWASP**: A03:2021 - Injection + +... + +## Medium Findings + +... + +## Remediation Priority + +1. **Critical (3)**: Fix within 24 hours +2. **High (8)**: Fix within 1 week +3. **Medium (10)**: Fix within 1 month +4. **Low (3)**: Fix in next release + +## Compliance Impact + +- **PCI DSS**: 4 findings affect compliance (SEC-001, SEC-002, SEC-008, SEC-011) +- **HIPAA**: 2 findings affect compliance (SEC-005, SEC-009) +- **GDPR**: 3 findings affect compliance (SEC-002, SEC-005, SEC-007) +``` + +## State Management + +```json +{ + "phase": "02-security-analysis", + "status": "completed", + "timestamp": "2024-01-15T11:15:00Z", + "input": { + "inventory_path": ".code-review/inventory.json", + "files_analyzed": 57 + }, + "output": { + "findings_path": ".code-review/security-findings.json", + "total_findings": 24, + "critical_count": 3, + "high_count": 8 + } +} +``` + +## Agent Instructions + +```markdown +You are in Phase 2 of the Code Review workflow. Your task is to analyze code for security vulnerabilities. + +**Instructions**: +1. Load file inventory from Phase 1 +2. Focus on Critical + High priority files +3. Run security checks for: + - OWASP Top 10 vulnerabilities + - CWE Top 25 weaknesses + - Language-specific security patterns +4. Use smart_search with mode="ripgrep" for pattern matching +5. Use mcp__ace-tool__search_context for semantic security pattern discovery +6. Classify findings by severity (Critical/High/Medium/Low) +7. Generate security-findings.json and markdown report +8. Proceed to Phase 3 (Best Practices Review) + +**Tools Available**: +- mcp__ccw_tools__smart_search (pattern search) +- mcp__ace-tool__search_context (semantic search) +- Read (read file content) +- Write (save findings and reports) +- Grep (targeted pattern matching) + +**Output Requirements**: +- security-findings.json with detailed findings +- Security report in markdown format +- Each finding must include: file, line, severity, CWE, OWASP, fix recommendation +- State file for phase tracking +``` + +## Validation + +Before proceeding to Phase 3: + +- ✅ All Critical + High priority files analyzed +- ✅ Findings categorized by severity +- ✅ Each finding has fix recommendation +- ✅ CWE and OWASP mappings included +- ✅ Security report generated +- ✅ State saved + +## Next Phase + +**Phase 3: Best Practices Review** - Analyze code quality, performance, and maintainability issues. diff --git a/.claude/skills/code-reviewer/phases/03-best-practices-review.md b/.claude/skills/code-reviewer/phases/03-best-practices-review.md new file mode 100644 index 00000000..f062300e --- /dev/null +++ b/.claude/skills/code-reviewer/phases/03-best-practices-review.md @@ -0,0 +1,36 @@ +# Phase 3: Best Practices Review + +## Objective + +Analyze code for best practices violations including code quality, performance issues, and maintainability concerns. + +## Input + +- **File Inventory**: From Phase 1 (`.code-review/inventory.json`) +- **Security Findings**: From Phase 2 (`.code-review/security-findings.json`) +- **User Arguments**: + - `--focus best-practices`: Best practices only mode + - `--check quality,performance,maintainability`: Specific areas to check + +## Process + +### Step 1: Code Quality Analysis + +Check naming conventions, function complexity, code duplication, and dead code detection. + +### Step 2: Performance Analysis + +Detect N+1 queries, inefficient algorithms, and memory leaks. + +### Step 3: Maintainability Analysis + +Check documentation coverage, test coverage, and dependency management. + +## Output + +- best-practices-findings.json +- Markdown report with recommendations + +## Next Phase + +**Phase 4: Report Generation** diff --git a/.claude/skills/code-reviewer/phases/04-report-generation.md b/.claude/skills/code-reviewer/phases/04-report-generation.md new file mode 100644 index 00000000..92bd9f3c --- /dev/null +++ b/.claude/skills/code-reviewer/phases/04-report-generation.md @@ -0,0 +1,278 @@ +# Phase 4: Report Generation + +## Objective + +Consolidate security and best practices findings into a comprehensive, actionable code review report. + +## Input + +- **Security Findings**: `.code-review/security-findings.json` +- **Best Practices Findings**: `.code-review/best-practices-findings.json` +- **File Inventory**: `.code-review/inventory.json` + +## Process + +### Step 1: Load All Findings + +```javascript +const securityFindings = JSON.parse( + await Read({ file_path: '.code-review/security-findings.json' }) +); +const bestPracticesFindings = JSON.parse( + await Read({ file_path: '.code-review/best-practices-findings.json' }) +); +const inventory = JSON.parse( + await Read({ file_path: '.code-review/inventory.json' }) +); +``` + +### Step 2: Aggregate Statistics + +```javascript +const stats = { + total_files_reviewed: inventory.total_files, + total_findings: securityFindings.total_findings + bestPracticesFindings.total_findings, + by_severity: { + critical: securityFindings.by_severity.critical, + high: securityFindings.by_severity.high + bestPracticesFindings.by_severity.high, + medium: securityFindings.by_severity.medium + bestPracticesFindings.by_severity.medium, + low: securityFindings.by_severity.low + bestPracticesFindings.by_severity.low, + }, + by_category: { + security: securityFindings.total_findings, + code_quality: bestPracticesFindings.by_category.code_quality, + performance: bestPracticesFindings.by_category.performance, + maintainability: bestPracticesFindings.by_category.maintainability, + } +}; +``` + +### Step 3: Generate Comprehensive Report + +```markdown +# Comprehensive Code Review Report + +**Generated**: {timestamp} +**Scope**: {scope} +**Files Reviewed**: {total_files} +**Total Findings**: {total_findings} + +## Executive Summary + +{Provide high-level overview of code health} + +### Risk Assessment + +{Calculate risk score based on findings} + +### Compliance Status + +{Map findings to compliance requirements} + +## Detailed Findings + +{Merge and organize security + best practices findings} + +## Action Plan + +{Prioritized list of fixes with effort estimates} + +## Appendix + +{Technical details, references, configuration} +``` + +### Step 4: Generate Fix Tracking Checklist + +Create actionable checklist for developers: + +```markdown +# Code Review Fix Checklist + +## Critical Issues (Fix Immediately) + +- [ ] [SEC-001] SQL Injection in src/auth/user-service.ts:145 +- [ ] [SEC-002] Hardcoded JWT Secret in src/auth/jwt.ts:23 +- [ ] [SEC-003] XSS Vulnerability in src/api/comments.ts:89 + +## High Priority Issues (Fix This Week) + +- [ ] [SEC-004] Missing Authorization Check in src/api/admin.ts:34 +- [ ] [BP-001] N+1 Query Pattern in src/api/orders.ts:45 +... +``` + +### Step 5: Generate Metrics Dashboard + +```markdown +## Code Health Metrics + +### Security Score: 68/100 +- Critical Issues: 3 (-30 points) +- High Issues: 8 (-2 points each) + +### Code Quality Score: 75/100 +- High Complexity Functions: 2 +- Code Duplication: 5% +- Dead Code: 3 instances + +### Performance Score: 82/100 +- N+1 Queries: 3 +- Inefficient Algorithms: 2 + +### Maintainability Score: 70/100 +- Documentation Coverage: 65% +- Test Coverage: 72% +- Missing Tests: 5 files +``` + +## Output + +### Main Report + +Save to `.code-review/REPORT.md`: + +- Executive summary +- Detailed findings (security + best practices) +- Action plan with priorities +- Metrics and scores +- References and compliance mapping + +### Fix Checklist + +Save to `.code-review/FIX-CHECKLIST.md`: + +- Organized by severity +- Checkboxes for tracking +- File:line references +- Effort estimates + +### JSON Summary + +Save to `.code-review/summary.json`: + +```json +{ + "report_date": "2024-01-15T12:00:00Z", + "scope": "src/**/*", + "statistics": { + "total_files": 247, + "total_findings": 69, + "by_severity": { "critical": 3, "high": 13, "medium": 30, "low": 23 }, + "by_category": { + "security": 24, + "code_quality": 18, + "performance": 12, + "maintainability": 15 + } + }, + "scores": { + "security": 68, + "code_quality": 75, + "performance": 82, + "maintainability": 70, + "overall": 74 + }, + "risk_level": "MEDIUM", + "action_required": true +} +``` + +## Report Template + +Full report includes: + +1. **Executive Summary** + - Overall code health + - Risk assessment + - Key recommendations + +2. **Security Findings** (from Phase 2) + - Critical/High/Medium/Low + - OWASP/CWE mappings + - Fix recommendations with code examples + +3. **Best Practices Findings** (from Phase 3) + - Code quality issues + - Performance concerns + - Maintainability gaps + +4. **Metrics Dashboard** + - Security score + - Code quality score + - Performance score + - Maintainability score + +5. **Action Plan** + - Immediate actions (critical) + - Short-term (1 week) + - Medium-term (1 month) + - Long-term (3 months) + +6. **Compliance Impact** + - PCI DSS findings + - HIPAA findings + - GDPR findings + - SOC 2 findings + +7. **Appendix** + - Full findings list + - Configuration used + - Tools and versions + - References + +## State Management + +```json +{ + "phase": "04-report-generation", + "status": "completed", + "timestamp": "2024-01-15T12:00:00Z", + "input": { + "security_findings": ".code-review/security-findings.json", + "best_practices_findings": ".code-review/best-practices-findings.json" + }, + "output": { + "report": ".code-review/REPORT.md", + "checklist": ".code-review/FIX-CHECKLIST.md", + "summary": ".code-review/summary.json" + } +} +``` + +## Agent Instructions + +```markdown +You are in Phase 4 (FINAL) of the Code Review workflow. Generate comprehensive report. + +**Instructions**: +1. Load security findings from Phase 2 +2. Load best practices findings from Phase 3 +3. Aggregate statistics and calculate scores +4. Generate comprehensive markdown report +5. Create fix tracking checklist +6. Generate JSON summary +7. Inform user of completion and output locations + +**Tools Available**: +- Read (load findings) +- Write (save reports) + +**Output Requirements**: +- REPORT.md (comprehensive markdown report) +- FIX-CHECKLIST.md (actionable checklist) +- summary.json (machine-readable summary) +- All files in .code-review/ directory +``` + +## Validation + +- ✅ All findings consolidated +- ✅ Scores calculated +- ✅ Action plan generated +- ✅ Reports saved to .code-review/ +- ✅ User notified of completion + +## Completion + +Code review complete! Outputs available in `.code-review/` directory. diff --git a/.claude/skills/code-reviewer/specs/best-practices-requirements.md b/.claude/skills/code-reviewer/specs/best-practices-requirements.md new file mode 100644 index 00000000..5f727613 --- /dev/null +++ b/.claude/skills/code-reviewer/specs/best-practices-requirements.md @@ -0,0 +1,346 @@ +# Best Practices Requirements Specification + +## Code Quality Standards + +### Naming Conventions + +**TypeScript/JavaScript**: +- Classes/Interfaces: PascalCase (`UserService`, `IUserRepository`) +- Functions/Methods: camelCase (`getUserById`, `validateEmail`) +- Constants: UPPER_SNAKE_CASE (`MAX_RETRY_COUNT`, `API_BASE_URL`) +- Private properties: prefix with `_` or `#` (`_cache`, `#secretKey`) + +**Python**: +- Classes: PascalCase (`UserService`, `DatabaseConnection`) +- Functions: snake_case (`get_user_by_id`, `validate_email`) +- Constants: UPPER_SNAKE_CASE (`MAX_RETRY_COUNT`) +- Private: prefix with `_` (`_internal_cache`) + +**Java**: +- Classes/Interfaces: PascalCase (`UserService`, `IUserRepository`) +- Methods: camelCase (`getUserById`, `validateEmail`) +- Constants: UPPER_SNAKE_CASE (`MAX_RETRY_COUNT`) +- Packages: lowercase (`com.example.service`) + +### Function Complexity + +**Cyclomatic Complexity Thresholds**: +- **Low**: 1-5 (simple functions, easy to test) +- **Medium**: 6-10 (acceptable, well-structured) +- **High**: 11-20 (needs refactoring) +- **Very High**: 21+ (critical, must refactor) + +**Calculation**: +``` +Complexity = 1 (base) + + count(if) + + count(else if) + + count(while) + + count(for) + + count(case) + + count(catch) + + count(&&) + + count(||) + + count(? :) +``` + +### Code Duplication + +**Thresholds**: +- **Acceptable**: < 3% duplication +- **Warning**: 3-5% duplication +- **Critical**: > 5% duplication + +**Detection**: +- Minimum block size: 5 lines +- Similarity threshold: 85% +- Ignore: Comments, imports, trivial getters/setters + +### Dead Code Detection + +**Targets**: +- Unused imports +- Unused variables/functions (not exported) +- Unreachable code (after return/throw) +- Commented-out code blocks (> 5 lines) + +## Performance Standards + +### N+1 Query Prevention + +**Anti-patterns**: +```javascript +// ❌ N+1 Query +for (const order of orders) { + const user = await User.findById(order.userId); +} + +// ✅ Batch Query +const userIds = orders.map(o => o.userId); +const users = await User.findByIds(userIds); +``` + +### Algorithm Efficiency + +**Common Issues**: +- Nested loops (O(n²)) when O(n) possible +- Array.indexOf in loop → use Set.has() +- Array.filter().length → use Array.some() +- Multiple array iterations → combine into one pass + +**Acceptable Complexity**: +- **O(1)**: Ideal for lookups +- **O(log n)**: Good for search +- **O(n)**: Acceptable for linear scan +- **O(n log n)**: Acceptable for sorting +- **O(n²)**: Avoid if possible, document if necessary + +### Memory Leak Prevention + +**Common Issues**: +- Event listeners without cleanup +- setInterval without clearInterval +- Global variable accumulation +- Circular references +- Large array/object allocations + +**Patterns**: +```javascript +// ❌ Memory Leak +element.addEventListener('click', handler); +// No cleanup + +// ✅ Proper Cleanup +useEffect(() => { + element.addEventListener('click', handler); + return () => element.removeEventListener('click', handler); +}, []); +``` + +### Resource Cleanup + +**Required Cleanup**: +- Database connections +- File handles +- Network sockets +- Timers (setTimeout, setInterval) +- Event listeners + +## Maintainability Standards + +### Documentation Requirements + +**Required for**: +- All exported functions/classes +- Public APIs +- Complex algorithms +- Non-obvious business logic + +**JSDoc Format**: +```javascript +/** + * Validates user credentials and generates JWT token + * + * @param {string} username - User's username or email + * @param {string} password - Plain text password + * @returns {Promise<{token: string, expiresAt: Date}>} JWT token and expiration + * @throws {AuthenticationError} If credentials are invalid + * + * @example + * const {token} = await authenticateUser('john@example.com', 'secret123'); + */ +async function authenticateUser(username, password) { + // ... +} +``` + +**Coverage Targets**: +- Critical modules: 100% +- High priority: 90% +- Medium priority: 70% +- Low priority: 50% + +### Test Coverage Requirements + +**Coverage Targets**: +- Unit tests: 80% line coverage +- Integration tests: Key workflows covered +- E2E tests: Critical user paths covered + +**Required Tests**: +- All exported functions +- All public methods +- Error handling paths +- Edge cases + +**Test File Convention**: +``` +src/auth/login.ts + → src/auth/login.test.ts (unit) + → src/auth/login.integration.test.ts (integration) +``` + +### Dependency Management + +**Best Practices**: +- Pin major versions (`"^1.2.3"` not `"*"`) +- Avoid 0.x versions in production +- Regular security audits (npm audit, snyk) +- Keep dependencies up-to-date +- Minimize dependency count + +**Version Pinning**: +```json +{ + "dependencies": { + "express": "^4.18.0", // ✅ Pinned major version + "lodash": "*", // ❌ Wildcard + "legacy-lib": "^0.5.0" // ⚠️ Unstable 0.x + } +} +``` + +### Magic Numbers + +**Definition**: Numeric literals without clear meaning + +**Anti-patterns**: +```javascript +// ❌ Magic numbers +if (user.age > 18) { } +setTimeout(() => {}, 5000); +buffer = new Array(1048576); + +// ✅ Named constants +const LEGAL_AGE = 18; +const RETRY_DELAY_MS = 5000; +const BUFFER_SIZE_1MB = 1024 * 1024; + +if (user.age > LEGAL_AGE) { } +setTimeout(() => {}, RETRY_DELAY_MS); +buffer = new Array(BUFFER_SIZE_1MB); +``` + +**Exceptions** (acceptable magic numbers): +- 0, 1, -1 (common values) +- 100, 1000 (obvious scaling factors in context) +- HTTP status codes (200, 404, 500) + +## Error Handling Standards + +### Required Error Handling + +**Categories**: +- Network errors (timeout, connection failure) +- Database errors (query failure, constraint violation) +- Validation errors (invalid input) +- Authentication/Authorization errors + +**Anti-patterns**: +```javascript +// ❌ Silent failure +try { + await saveUser(user); +} catch (err) { + // Empty catch +} + +// ❌ Generic catch +try { + await processPayment(order); +} catch (err) { + console.log('Error'); // No details +} + +// ✅ Proper handling +try { + await processPayment(order); +} catch (err) { + logger.error('Payment processing failed', { orderId: order.id, error: err }); + throw new PaymentError('Failed to process payment', { cause: err }); +} +``` + +### Logging Standards + +**Required Logs**: +- Authentication attempts (success/failure) +- Authorization failures +- Data modifications (create/update/delete) +- External API calls +- Errors and exceptions + +**Log Levels**: +- **ERROR**: System errors, exceptions +- **WARN**: Recoverable issues, deprecations +- **INFO**: Business events, state changes +- **DEBUG**: Detailed troubleshooting info + +**Sensitive Data**: +- Never log: passwords, tokens, credit cards, SSNs +- Hash/mask: emails, IPs, usernames (in production) + +## Code Structure Standards + +### File Organization + +**Max File Size**: 300 lines (excluding tests) +**Max Function Size**: 50 lines + +**Module Structure**: +``` +module/ + ├── index.ts # Public exports + ├── types.ts # Type definitions + ├── constants.ts # Constants + ├── utils.ts # Utilities + ├── service.ts # Business logic + └── service.test.ts # Tests +``` + +### Import Organization + +**Order**: +1. External dependencies +2. Internal modules (absolute imports) +3. Relative imports +4. Type imports (TypeScript) + +```typescript +// ✅ Organized imports +import express from 'express'; +import { Logger } from 'winston'; + +import { UserService } from '@/services/user'; +import { config } from '@/config'; + +import { validateEmail } from './utils'; +import { UserRepository } from './repository'; + +import type { User, UserCreateInput } from './types'; +``` + +## Scoring System + +### Overall Score Calculation + +``` +Overall Score = ( + Security Score × 0.4 + + Code Quality Score × 0.25 + + Performance Score × 0.2 + + Maintainability Score × 0.15 +) + +Security = 100 - (Critical × 30 + High × 2 + Medium × 0.5) +Code Quality = 100 - (violations / total_checks × 100) +Performance = 100 - (issues / potential_issues × 100) +Maintainability = (doc_coverage × 0.4 + test_coverage × 0.4 + dependency_health × 0.2) +``` + +### Risk Levels + +- **LOW**: Score 90-100 +- **MEDIUM**: Score 70-89 +- **HIGH**: Score 50-69 +- **CRITICAL**: Score < 50 diff --git a/.claude/skills/code-reviewer/specs/quality-standards.md b/.claude/skills/code-reviewer/specs/quality-standards.md new file mode 100644 index 00000000..92fba784 --- /dev/null +++ b/.claude/skills/code-reviewer/specs/quality-standards.md @@ -0,0 +1,252 @@ +# Quality Standards + +## Overall Quality Metrics + +### Quality Score Formula + +``` +Overall Quality = ( + Correctness × 0.30 + + Security × 0.25 + + Maintainability × 0.20 + + Performance × 0.15 + + Documentation × 0.10 +) +``` + +### Score Ranges + +| Range | Grade | Description | +|-------|-------|-------------| +| 90-100 | A | Excellent - Production ready | +| 80-89 | B | Good - Minor improvements needed | +| 70-79 | C | Acceptable - Some issues to address | +| 60-69 | D | Poor - Significant improvements required | +| 0-59 | F | Failing - Major issues, not production ready | + +## Review Completeness + +### Mandatory Checks + +**Security**: +- ✅ OWASP Top 10 coverage +- ✅ CWE Top 25 coverage +- ✅ Language-specific security patterns +- ✅ Dependency vulnerability scan + +**Code Quality**: +- ✅ Naming convention compliance +- ✅ Complexity analysis +- ✅ Code duplication detection +- ✅ Dead code identification + +**Performance**: +- ✅ N+1 query detection +- ✅ Algorithm efficiency check +- ✅ Memory leak detection +- ✅ Resource cleanup verification + +**Maintainability**: +- ✅ Documentation coverage +- ✅ Test coverage analysis +- ✅ Dependency health check +- ✅ Error handling review + +## Reporting Standards + +### Finding Requirements + +Each finding must include: +- **Unique ID**: SEC-001, BP-001, etc. +- **Type**: Specific issue type (sql-injection, high-complexity, etc.) +- **Severity**: Critical, High, Medium, Low +- **Location**: File path and line number +- **Code Snippet**: Vulnerable/problematic code +- **Message**: Clear description of the issue +- **Recommendation**: Specific fix guidance +- **Example**: Before/after code example + +### Report Structure + +**Executive Summary**: +- High-level overview +- Risk assessment +- Key statistics +- Compliance status + +**Detailed Findings**: +- Organized by severity +- Grouped by category +- Full details for each finding + +**Action Plan**: +- Prioritized fix list +- Effort estimates +- Timeline recommendations + +**Metrics Dashboard**: +- Quality scores +- Trend analysis (if historical data) +- Compliance status + +**Appendix**: +- Full findings list +- Configuration details +- Tool versions +- References + +## Output File Standards + +### File Naming + +``` +.code-review/ +├── inventory.json # File inventory +├── security-findings.json # Security findings +├── best-practices-findings.json # Best practices findings +├── summary.json # Summary statistics +├── REPORT.md # Main report +├── FIX-CHECKLIST.md # Action checklist +└── state.json # Session state +``` + +### JSON Schema + +**Finding Schema**: +```json +{ + "id": "string", + "type": "string", + "category": "security|code_quality|performance|maintainability", + "severity": "critical|high|medium|low", + "file": "string", + "line": "number", + "column": "number", + "code": "string", + "message": "string", + "recommendation": { + "description": "string", + "fix_example": "string" + }, + "references": ["string"], + "cwe": "string (optional)", + "owasp": "string (optional)" +} +``` + +## Validation Requirements + +### Phase Completion Criteria + +**Phase 1 (Code Discovery)**: +- ✅ At least 1 file discovered +- ✅ Files categorized by priority +- ✅ Metadata extracted +- ✅ Inventory JSON created + +**Phase 2 (Security Analysis)**: +- ✅ All critical/high priority files analyzed +- ✅ Findings have severity classification +- ✅ CWE/OWASP mappings included +- ✅ Fix recommendations provided + +**Phase 3 (Best Practices)**: +- ✅ Code quality checks completed +- ✅ Performance analysis done +- ✅ Maintainability assessed +- ✅ Recommendations provided + +**Phase 4 (Report Generation)**: +- ✅ All findings consolidated +- ✅ Scores calculated +- ✅ Reports generated +- ✅ Checklist created + +## Skill Execution Standards + +### Performance Targets + +- **Phase 1**: < 30 seconds per 1000 files +- **Phase 2**: < 60 seconds per 100 files (security) +- **Phase 3**: < 60 seconds per 100 files (best practices) +- **Phase 4**: < 10 seconds (report generation) + +### Resource Limits + +- **Memory**: < 2GB for projects with 1000+ files +- **CPU**: Efficient pattern matching (minimize regex complexity) +- **Disk**: Use streaming for large files (> 10MB) + +### Error Handling + +**Graceful Degradation**: +- If tool unavailable: Skip check, note in report +- If file unreadable: Log warning, continue with others +- If analysis fails: Report error, continue with next file + +**User Notification**: +- Progress updates every 10% completion +- Clear error messages with troubleshooting steps +- Final summary with metrics and file locations + +## Integration Standards + +### Git Integration + +**Pre-commit Hook**: +```bash +#!/bin/bash +ccw run code-reviewer --scope staged --severity critical,high +exit $? # Block commit if critical/high issues found +``` + +**PR Comments**: +- Automatic review comments on changed lines +- Summary comment with overall findings +- Status check (pass/fail based on threshold) + +### CI/CD Integration + +**Requirements**: +- Exit code 0 if no critical/high issues +- Exit code 1 if blocking issues found +- JSON output for parsing +- Configurable severity threshold + +### IDE Integration + +**LSP Support** (future): +- Real-time security/quality feedback +- Inline fix suggestions +- Quick actions for common fixes + +## Compliance Mapping + +### Supported Standards + +**PCI DSS**: +- Requirement 6.5: Common coding vulnerabilities +- Map findings to specific requirements + +**HIPAA**: +- Technical safeguards +- Map data exposure findings + +**GDPR**: +- Data protection by design +- Map sensitive data handling + +**SOC 2**: +- Security controls +- Map access control findings + +### Compliance Reports + +Generate compliance-specific reports: +``` +.code-review/compliance/ +├── pci-dss-report.md +├── hipaa-report.md +├── gdpr-report.md +└── soc2-report.md +``` diff --git a/.claude/skills/code-reviewer/specs/security-requirements.md b/.claude/skills/code-reviewer/specs/security-requirements.md new file mode 100644 index 00000000..59458f70 --- /dev/null +++ b/.claude/skills/code-reviewer/specs/security-requirements.md @@ -0,0 +1,243 @@ +# Security Requirements Specification + +## OWASP Top 10 Coverage + +### A01:2021 - Broken Access Control + +**Checks**: +- Missing authorization checks on protected routes +- Insecure direct object references (IDOR) +- Path traversal vulnerabilities +- Missing CSRF protection +- Elevation of privilege + +**Patterns**: +```javascript +// Missing auth middleware +router.get('/admin/*', handler); // ❌ No auth check + +// Insecure direct object reference +router.get('/user/:id', async (req, res) => { + const user = await User.findById(req.params.id); // ❌ No ownership check + res.json(user); +}); +``` + +### A02:2021 - Cryptographic Failures + +**Checks**: +- Sensitive data transmitted without encryption +- Weak cryptographic algorithms (MD5, SHA1) +- Hardcoded secrets/keys +- Insecure random number generation + +**Patterns**: +```javascript +// Weak hashing +const hash = crypto.createHash('md5').update(password); // ❌ MD5 is weak + +// Hardcoded secret +const token = jwt.sign(payload, 'secret123'); // ❌ Hardcoded secret +``` + +### A03:2021 - Injection + +**Checks**: +- SQL injection +- NoSQL injection +- Command injection +- LDAP injection +- XPath injection + +**Patterns**: +```javascript +// SQL injection +const query = `SELECT * FROM users WHERE id = ${userId}`; // ❌ + +// Command injection +exec(`git clone ${userRepo}`); // ❌ +``` + +### A04:2021 - Insecure Design + +**Checks**: +- Missing rate limiting +- Lack of input validation +- Business logic flaws +- Missing security requirements + +### A05:2021 - Security Misconfiguration + +**Checks**: +- Default credentials +- Overly permissive CORS +- Verbose error messages +- Unnecessary features enabled +- Missing security headers + +**Patterns**: +```javascript +// Overly permissive CORS +app.use(cors({ origin: '*' })); // ❌ + +// Verbose error +res.status(500).json({ error: err.stack }); // ❌ +``` + +### A06:2021 - Vulnerable and Outdated Components + +**Checks**: +- Dependencies with known vulnerabilities +- Unmaintained dependencies +- Using deprecated APIs + +### A07:2021 - Identification and Authentication Failures + +**Checks**: +- Weak password requirements +- Permits brute force attacks +- Exposed session IDs +- Weak JWT implementation + +**Patterns**: +```javascript +// Weak bcrypt rounds +bcrypt.hash(password, 4); // ❌ Too low (min: 10) + +// Session ID in URL +res.redirect(`/dashboard?sessionId=${sessionId}`); // ❌ +``` + +### A08:2021 - Software and Data Integrity Failures + +**Checks**: +- Insecure deserialization +- Unsigned/unverified updates +- CI/CD pipeline vulnerabilities + +**Patterns**: +```javascript +// Insecure deserialization +const obj = eval(userInput); // ❌ + +// Pickle vulnerability (Python) +data = pickle.loads(untrusted_data) # ❌ +``` + +### A09:2021 - Security Logging and Monitoring Failures + +**Checks**: +- Missing audit logs +- Sensitive data in logs +- Insufficient monitoring + +**Patterns**: +```javascript +// Password in logs +console.log(`Login attempt: ${username}:${password}`); // ❌ +``` + +### A10:2021 - Server-Side Request Forgery (SSRF) + +**Checks**: +- Unvalidated URLs in requests +- Internal network access +- Cloud metadata exposure + +**Patterns**: +```javascript +// SSRF vulnerability +const response = await fetch(userProvidedUrl); // ❌ +``` + +## CWE Top 25 Coverage + +### CWE-79: Cross-site Scripting (XSS) + +**Patterns**: +```javascript +element.innerHTML = userInput; // ❌ +document.write(userInput); // ❌ +``` + +### CWE-89: SQL Injection + +**Patterns**: +```javascript +query = `SELECT * FROM users WHERE name = '${name}'`; // ❌ +``` + +### CWE-20: Improper Input Validation + +**Checks**: +- Missing input sanitization +- No input length limits +- Unvalidated file uploads + +### CWE-78: OS Command Injection + +**Patterns**: +```javascript +exec(`ping ${userInput}`); // ❌ +``` + +### CWE-190: Integer Overflow + +**Checks**: +- Large number operations without bounds checking +- Array allocation with user-controlled size + +## Language-Specific Security Rules + +### TypeScript/JavaScript + +- Prototype pollution +- eval() usage +- Unsafe regex (ReDoS) +- require() with dynamic input + +### Python + +- pickle vulnerabilities +- yaml.unsafe_load() +- SQL injection in SQLAlchemy +- Command injection in subprocess + +### Java + +- Deserialization vulnerabilities +- XXE in XML parsers +- Path traversal +- SQL injection in JDBC + +### Go + +- Race conditions +- SQL injection +- Path traversal +- Weak cryptography + +## Severity Classification + +### Critical +- Remote code execution +- SQL injection with write access +- Authentication bypass +- Hardcoded credentials in production + +### High +- XSS in sensitive contexts +- Missing authorization checks +- Sensitive data exposure +- Insecure cryptography + +### Medium +- Missing rate limiting +- Weak password policy +- Security misconfiguration +- Information disclosure + +### Low +- Missing security headers +- Verbose error messages +- Outdated dependencies (no known exploits) diff --git a/.claude/skills/code-reviewer/templates/best-practice-finding.md b/.claude/skills/code-reviewer/templates/best-practice-finding.md new file mode 100644 index 00000000..bb3424c2 --- /dev/null +++ b/.claude/skills/code-reviewer/templates/best-practice-finding.md @@ -0,0 +1,234 @@ +# Best Practice Finding Template + +Use this template for documenting code quality, performance, and maintainability issues. + +## Finding Structure + +```json +{ + "id": "BP-{number}", + "type": "{issue-type}", + "category": "{code_quality|performance|maintainability}", + "severity": "{high|medium|low}", + "file": "{file-path}", + "line": {line-number}, + "function": "{function-name}", + "message": "{clear-description}", + "recommendation": { + "description": "{how-to-fix}", + "example": "{corrected-code}" + } +} +``` + +## Markdown Template + +```markdown +### 🟠 [BP-{number}] {Issue Title} + +**File**: `{file-path}:{line}` +**Category**: {Code Quality|Performance|Maintainability} + +**Issue**: {Detailed explanation of the problem} + +**Current Code**: +\`\`\`{language} +{problematic-code} +\`\`\` + +**Recommended Fix**: +\`\`\`{language} +{improved-code-with-comments} +\`\`\` + +**Impact**: {Why this matters - readability, performance, maintainability} + +--- +``` + +## Example: High Complexity + +```markdown +### 🟠 [BP-001] High Cyclomatic Complexity + +**File**: `src/utils/validator.ts:78` +**Category**: Code Quality +**Function**: `validateUserInput` +**Complexity**: 15 (threshold: 10) + +**Issue**: Function has 15 decision points, making it difficult to test and maintain. + +**Current Code**: +\`\`\`typescript +function validateUserInput(input) { + if (!input) return false; + if (!input.email) return false; + if (!input.email.includes('@')) return false; + if (input.email.length > 255) return false; + // ... 11 more conditions +} +\`\`\` + +**Recommended Fix**: +\`\`\`typescript +// Extract validation rules +const validationRules = { + email: (email) => email && email.includes('@') && email.length <= 255, + password: (pwd) => pwd && pwd.length >= 8 && /[A-Z]/.test(pwd), + username: (name) => name && /^[a-zA-Z0-9_]+$/.test(name), +}; + +// Simplified validator +function validateUserInput(input) { + return Object.entries(validationRules).every(([field, validate]) => + validate(input[field]) + ); +} +\`\`\` + +**Impact**: Reduces complexity from 15 to 3, improves testability, and makes validation rules reusable. + +--- +``` + +## Example: N+1 Query + +```markdown +### 🟠 [BP-002] N+1 Query Pattern + +**File**: `src/api/orders.ts:45` +**Category**: Performance + +**Issue**: Database query executed inside loop, causing N+1 queries problem. For 100 orders, this creates 101 database queries instead of 2. + +**Current Code**: +\`\`\`typescript +const orders = await Order.findAll(); +for (const order of orders) { + const user = await User.findById(order.userId); + order.userName = user.name; +} +\`\`\` + +**Recommended Fix**: +\`\`\`typescript +// Batch query all users at once +const orders = await Order.findAll(); +const userIds = orders.map(o => o.userId); +const users = await User.findByIds(userIds); + +// Create lookup map for O(1) access +const userMap = new Map(users.map(u => [u.id, u])); + +// Enrich orders with user data +for (const order of orders) { + order.userName = userMap.get(order.userId)?.name; +} +\`\`\` + +**Impact**: Reduces database queries from O(n) to O(1), significantly improving performance for large datasets. + +--- +``` + +## Example: Missing Documentation + +```markdown +### 🟡 [BP-003] Missing Documentation + +**File**: `src/services/PaymentService.ts:23` +**Category**: Maintainability + +**Issue**: Exported class lacks documentation, making it difficult for other developers to understand its purpose and usage. + +**Current Code**: +\`\`\`typescript +export class PaymentService { + async processPayment(orderId: string, amount: number) { + // implementation + } +} +\`\`\` + +**Recommended Fix**: +\`\`\`typescript +/** + * Service for processing payment transactions + * + * Handles payment processing, refunds, and transaction logging. + * Integrates with Stripe payment gateway. + * + * @example + * const paymentService = new PaymentService(); + * const result = await paymentService.processPayment('order-123', 99.99); + */ +export class PaymentService { + /** + * Process a payment for an order + * + * @param orderId - Unique order identifier + * @param amount - Payment amount in USD + * @returns Payment confirmation with transaction ID + * @throws {PaymentError} If payment processing fails + */ + async processPayment(orderId: string, amount: number) { + // implementation + } +} +\`\`\` + +**Impact**: Improves code discoverability and reduces onboarding time for new developers. + +--- +``` + +## Example: Memory Leak + +```markdown +### 🟠 [BP-004] Potential Memory Leak + +**File**: `src/components/Chat.tsx:56` +**Category**: Performance + +**Issue**: WebSocket event listener added without cleanup, causing memory leaks when component unmounts. + +**Current Code**: +\`\`\`tsx +useEffect(() => { + socket.on('message', handleMessage); +}, []); +\`\`\` + +**Recommended Fix**: +\`\`\`tsx +useEffect(() => { + socket.on('message', handleMessage); + + // Cleanup on unmount + return () => { + socket.off('message', handleMessage); + }; +}, []); +\`\`\` + +**Impact**: Prevents memory leaks and improves application stability in long-running sessions. + +--- +``` + +## Severity Guidelines + +### High +- Major performance impact (N+1 queries, O(n²) algorithms) +- Critical maintainability issues (complexity > 15) +- Missing error handling in critical paths + +### Medium +- Moderate performance impact +- Code quality issues (complexity 11-15, duplication) +- Missing tests for important features + +### Low +- Minor style violations +- Missing documentation +- Low-impact dead code diff --git a/.claude/skills/code-reviewer/templates/report-template.md b/.claude/skills/code-reviewer/templates/report-template.md new file mode 100644 index 00000000..4d159c49 --- /dev/null +++ b/.claude/skills/code-reviewer/templates/report-template.md @@ -0,0 +1,316 @@ +# Report Template + +## Main Report Structure (REPORT.md) + +```markdown +# Code Review Report + +**Generated**: {timestamp} +**Scope**: {scope} +**Files Reviewed**: {total_files} +**Total Findings**: {total_findings} + +--- + +## 📊 Executive Summary + +### Overall Assessment + +{Brief 2-3 paragraph assessment of code health} + +### Risk Level: {LOW|MEDIUM|HIGH|CRITICAL} + +{Risk assessment based on findings severity and count} + +### Key Statistics + +| Metric | Value | Status | +|--------|-------|--------| +| Total Files | {count} | - | +| Files with Issues | {count} | {percentage}% | +| Critical Findings | {count} | {icon} | +| High Findings | {count} | {icon} | +| Medium Findings | {count} | {icon} | +| Low Findings | {count} | {icon} | + +### Category Breakdown + +| Category | Count | Percentage | +|----------|-------|------------| +| Security | {count} | {percentage}% | +| Code Quality | {count} | {percentage}% | +| Performance | {count} | {percentage}% | +| Maintainability | {count} | {percentage}% | + +--- + +## 🎯 Quality Scores + +### Security Score: {score}/100 +{Assessment and key issues} + +### Code Quality Score: {score}/100 +{Assessment and key issues} + +### Performance Score: {score}/100 +{Assessment and key issues} + +### Maintainability Score: {score}/100 +{Assessment and key issues} + +### Overall Score: {score}/100 + +**Grade**: {A|B|C|D|F} + +--- + +## 🔴 Critical Findings (Requires Immediate Action) + +{List all critical findings using security-finding.md template} + +--- + +## 🟠 High Priority Findings + +{List all high findings} + +--- + +## 🟡 Medium Priority Findings + +{List all medium findings} + +--- + +## 🟢 Low Priority Findings + +{List all low findings} + +--- + +## 📋 Action Plan + +### Immediate (Within 24 hours) +1. {Critical issue 1} +2. {Critical issue 2} +3. {Critical issue 3} + +### Short-term (Within 1 week) +1. {High priority issue 1} +2. {High priority issue 2} +... + +### Medium-term (Within 1 month) +1. {Medium priority issue 1} +2. {Medium priority issue 2} +... + +### Long-term (Within 3 months) +1. {Low priority issue 1} +2. {Improvement initiative 1} +... + +--- + +## 📊 Metrics Dashboard + +### Code Health Trends + +{If historical data available, show trends} + +### File Hotspots + +Top files with most issues: +1. `{file-path}` - {count} issues ({severity breakdown}) +2. `{file-path}` - {count} issues +... + +### Technology Breakdown + +Issues by language/framework: +- TypeScript: {count} issues +- Python: {count} issues +... + +--- + +## ✅ Compliance Status + +### PCI DSS +- **Status**: {COMPLIANT|NON-COMPLIANT|PARTIAL} +- **Affecting Findings**: {list} + +### HIPAA +- **Status**: {COMPLIANT|NON-COMPLIANT|PARTIAL} +- **Affecting Findings**: {list} + +### GDPR +- **Status**: {COMPLIANT|NON-COMPLIANT|PARTIAL} +- **Affecting Findings**: {list} + +--- + +## 📚 Appendix + +### A. Review Configuration + +\`\`\`json +{review-config} +\`\`\` + +### B. Tools and Versions + +- Code Reviewer Skill: v1.0.0 +- Security Rules: OWASP Top 10 2021, CWE Top 25 +- Languages Analyzed: {list} + +### C. References + +- [OWASP Top 10 2021](https://owasp.org/Top10/) +- [CWE Top 25](https://cwe.mitre.org/top25/) +- {additional references} + +### D. Full Findings Index + +{Links to detailed finding JSONs} +``` + +--- + +## Fix Checklist Template (FIX-CHECKLIST.md) + +```markdown +# Code Review Fix Checklist + +**Generated**: {timestamp} +**Total Items**: {count} + +--- + +## 🔴 Critical Issues (Fix Immediately) + +- [ ] **[SEC-001]** SQL Injection in `src/auth/user-service.ts:145` + - Effort: 1 hour + - Priority: P0 + - Assignee: ___________ + +- [ ] **[SEC-002]** Hardcoded JWT Secret in `src/auth/jwt.ts:23` + - Effort: 30 minutes + - Priority: P0 + - Assignee: ___________ + +--- + +## 🟠 High Priority Issues (Fix This Week) + +- [ ] **[SEC-003]** Missing Authorization in `src/api/admin.ts:34` + - Effort: 2 hours + - Priority: P1 + - Assignee: ___________ + +- [ ] **[BP-001]** N+1 Query in `src/api/orders.ts:45` + - Effort: 1 hour + - Priority: P1 + - Assignee: ___________ + +--- + +## 🟡 Medium Priority Issues (Fix This Month) + +{List medium priority items} + +--- + +## 🟢 Low Priority Issues (Fix Next Release) + +{List low priority items} + +--- + +## Progress Tracking + +**Overall Progress**: {completed}/{total} ({percentage}%) + +- Critical: {completed}/{total} +- High: {completed}/{total} +- Medium: {completed}/{total} +- Low: {completed}/{total} + +**Estimated Total Effort**: {hours} hours +**Estimated Completion**: {date} +``` + +--- + +## Summary JSON Template (summary.json) + +```json +{ + "report_date": "2024-01-15T12:00:00Z", + "scope": "src/**/*", + "statistics": { + "total_files": 247, + "files_with_issues": 89, + "total_findings": 69, + "by_severity": { + "critical": 3, + "high": 13, + "medium": 30, + "low": 23 + }, + "by_category": { + "security": 24, + "code_quality": 18, + "performance": 12, + "maintainability": 15 + } + }, + "scores": { + "security": 68, + "code_quality": 75, + "performance": 82, + "maintainability": 70, + "overall": 74 + }, + "grade": "C", + "risk_level": "MEDIUM", + "action_required": true, + "compliance": { + "pci_dss": { + "status": "NON_COMPLIANT", + "affecting_findings": ["SEC-001", "SEC-002", "SEC-008", "SEC-011"] + }, + "hipaa": { + "status": "NON_COMPLIANT", + "affecting_findings": ["SEC-005", "SEC-009"] + }, + "gdpr": { + "status": "PARTIAL", + "affecting_findings": ["SEC-002", "SEC-005", "SEC-007"] + } + }, + "top_issues": [ + { + "id": "SEC-001", + "type": "sql-injection", + "severity": "critical", + "file": "src/auth/user-service.ts", + "line": 145 + } + ], + "hotspots": [ + { + "file": "src/auth/user-service.ts", + "issues": 5, + "severity_breakdown": { "critical": 1, "high": 2, "medium": 2 } + } + ], + "effort_estimate": { + "critical": 4.5, + "high": 18, + "medium": 35, + "low": 12, + "total_hours": 69.5 + } +} +``` diff --git a/.claude/skills/code-reviewer/templates/security-finding.md b/.claude/skills/code-reviewer/templates/security-finding.md new file mode 100644 index 00000000..083c542a --- /dev/null +++ b/.claude/skills/code-reviewer/templates/security-finding.md @@ -0,0 +1,161 @@ +# Security Finding Template + +Use this template for documenting security vulnerabilities. + +## Finding Structure + +```json +{ + "id": "SEC-{number}", + "type": "{vulnerability-type}", + "severity": "{critical|high|medium|low}", + "file": "{file-path}", + "line": {line-number}, + "column": {column-number}, + "code": "{vulnerable-code-snippet}", + "message": "{clear-description-of-issue}", + "cwe": "CWE-{number}", + "owasp": "A{number}:2021 - {category}", + "recommendation": { + "description": "{how-to-fix}", + "fix_example": "{corrected-code}" + }, + "references": [ + "https://...", + "https://..." + ] +} +``` + +## Markdown Template + +```markdown +### 🔴 [SEC-{number}] {Vulnerability Title} + +**File**: `{file-path}:{line}` +**CWE**: CWE-{number} | **OWASP**: A{number}:2021 - {category} + +**Vulnerable Code**: +\`\`\`{language} +{vulnerable-code-snippet} +\`\`\` + +**Issue**: {Detailed explanation of the vulnerability and potential impact} + +**Attack Example** (if applicable): +\`\`\` +{example-attack-payload} +Result: {what-happens} +Effect: {security-impact} +\`\`\` + +**Recommended Fix**: +\`\`\`{language} +{corrected-code-with-comments} +\`\`\` + +**References**: +- [{reference-title}]({url}) +- [{reference-title}]({url}) + +--- +``` + +## Severity Icon Mapping + +- Critical: 🔴 +- High: 🟠 +- Medium: 🟡 +- Low: 🟢 + +## Example: SQL Injection Finding + +```markdown +### 🔴 [SEC-001] SQL Injection in User Authentication + +**File**: `src/auth/user-service.ts:145` +**CWE**: CWE-89 | **OWASP**: A03:2021 - Injection + +**Vulnerable Code**: +\`\`\`typescript +const query = \`SELECT * FROM users WHERE username = '\${username}'\`; +const user = await db.execute(query); +\`\`\` + +**Issue**: User input (`username`) is directly concatenated into SQL query, allowing attackers to inject malicious SQL commands and bypass authentication. + +**Attack Example**: +\`\`\` +username: ' OR '1'='1' -- +Result: SELECT * FROM users WHERE username = '' OR '1'='1' --' +Effect: Bypasses authentication, returns all users +\`\`\` + +**Recommended Fix**: +\`\`\`typescript +// Use parameterized queries +const query = 'SELECT * FROM users WHERE username = ?'; +const user = await db.execute(query, [username]); + +// Or use ORM +const user = await User.findOne({ where: { username } }); +\`\`\` + +**References**: +- [OWASP SQL Injection](https://owasp.org/www-community/attacks/SQL_Injection) +- [CWE-89](https://cwe.mitre.org/data/definitions/89.html) + +--- +``` + +## Example: XSS Finding + +```markdown +### 🟠 [SEC-002] Cross-Site Scripting (XSS) in Comment Rendering + +**File**: `src/components/CommentList.tsx:89` +**CWE**: CWE-79 | **OWASP**: A03:2021 - Injection + +**Vulnerable Code**: +\`\`\`tsx +
+\`\`\` + +**Issue**: User-generated content rendered without sanitization, allowing script injection. + +**Attack Example**: +\`\`\` +comment.body: "" +Effect: Steals user session cookies +\`\`\` + +**Recommended Fix**: +\`\`\`tsx +import DOMPurify from 'dompurify'; + +// Sanitize HTML before rendering + + +// Or use text content (if HTML not needed) +$1');
+
+ // Build type badge if has prefix
+ const typeBadge = parsed.hasPrefix ?
+ `
+
+ ${parsed.label}
+ ` : '';
+
+ // Determine line class based on original type and parsed type
+ const lineClass = parsed.hasPrefix ? `cli-stream-line formatted ${parsed.type}` :
+ `cli-stream-line ${line.type}`;
+
+ return `