code-review

What this skill does

# Code Review

> Comprehensive AI-powered code review for PRs and local changes — enterprise-grade alternative to CodeRabbit

## When to use

- "code review"
- "review my PR"
- "review PR #123"
- "check my changes"
- "what's wrong with my code"
- "security review"
- "full review"
- "/review"
- "/review-pr"

## Dependencies

- External: `gh` CLI (GitHub), `git`

## Modes

### 1. Local review (uncommitted changes)
Reviews `git diff` — changes not yet committed.

### 2. Branch review (vs main/master)
Reviews all changes in current branch compared to main.

### 3. PR review (GitHub)
Fetches diff from GitHub PR and can post comments.

### 4. Focused review
User can request specific focus: security, performance, bugs, style, etc.

---

## How to execute

### Step 0: Check if review needed

**Skip review if:**
- PR is draft (`gh pr view --json isDraft`)
- PR is already closed/merged
- Only documentation changes (.md, .txt, LICENSE)
- Only config changes (.json, .yaml, .toml) without code impact
- Trivial changes (<5 lines, whitespace only, version bumps)

**Inform user and ask to confirm if they still want review.**

---

### Step 1: Determine mode

Ask user or detect automatically:
- If PR number provided → PR review
- If uncommitted changes exist → local review
- If on feature branch → branch review
- If specific focus requested → apply focus filter

### Step 2: Get diff

**Local:**
```bash
git diff HEAD
```

**Branch (vs main):**
```bash
DEFAULT_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@' || echo "main")
git diff $DEFAULT_BRANCH...HEAD
```

**PR:**
```bash
gh pr diff <PR_NUMBER>
```

### Step 3: Get context

For thorough review, read related files:
```bash
# List changed files
git diff --name-only HEAD

# Read each file fully for context
# Check package.json for dependencies
# Check tsconfig/eslint config for project standards
```

### Step 3b: Filter pre-existing issues

Before reporting an issue, check if it was introduced in this PR:

```bash
# Check when the problematic line was last modified
git blame -L <start>,<end> <file> --porcelain | head -1
```

**Skip issues that:**
- Existed before this PR (old blame hash)
- Are in unchanged lines
- Were introduced by a different author long ago

**Only report issues introduced or modified in current changes.**

This prevents noise from legacy code and focuses review on new changes.

---

### Step 4: Comprehensive Analysis

Apply ALL relevant checks from the checklist below.

### Step 5: Confidence Scoring

**Rate each issue 0-100:**

| Score | Confidence | When to use |
|-------|------------|-------------|
| 90-100 | Certain | Clear vulnerability (SQL injection with user input), obvious crash |
| 70-89 | High | Likely bug, security risk, definite code smell |
| 50-69 | Medium | Potential issue, needs context to confirm |
| 25-49 | Low | Style preference, minor suggestion |
| 0-24 | Skip | Probably false positive, pre-existing, or nitpick |

**Only report issues with confidence ≥70.**

**Mark as false positive and skip:**
- Pre-existing issues (caught by Step 3b)
- Issues that linters will catch (eslint, prettier)
- Pedantic nitpicks without real impact
- Code that looks wrong but has valid reason (check comments)
- Issues with explicit ignore comments (`// eslint-disable`, `# noqa`)

---

### Step 6: Output result

**Format:**
```markdown
## Code Review Summary

**Reviewed:** X files, Y lines changed
**Risk Level:** Critical / High / Medium / Low

### Critical Issues (must fix)
- [file:line] Description — Why it matters

### High Priority
- [file:line] Description

### Medium Priority
- [file:line] Description

### Low Priority / Suggestions
- [file:line] Description

### Good Practices
- What was done well
```

**For GitHub PR — post comments:**
```bash
# General comment on PR
gh pr comment <PR_NUMBER> --body "## AI Code Review
[Review content]"

# Line-by-line comments via API (for specific file/line feedback)
# Replace {owner}, {repo}, {pr} with actual values
gh api repos/{owner}/{repo}/pulls/{pr}/comments \
  --method POST \
  -f body="Issue description and fix suggestion" \
  -f path="src/file.ts" \
  -f line=42 \
  -f side="RIGHT"
```

---

## COMPREHENSIVE REVIEW CHECKLIST

### 1. SECURITY (OWASP Top 10 + Extended)

#### 1.1 Injection
- [ ] **SQL Injection** — User input in SQL queries without parameterization
- [ ] **NoSQL Injection** — Unsanitized input in MongoDB/similar queries
- [ ] **Command Injection** — User input passed to shell commands (exec, spawn, system)
- [ ] **LDAP Injection** — User input in LDAP queries
- [ ] **XPath Injection** — User input in XML queries
- [ ] **Template Injection** — User input in template engines (SSTI)
- [ ] **Header Injection** — User input in HTTP headers (CRLF)
- [ ] **Log Injection** — Unsanitized data written to logs

#### 1.2 Broken Authentication
- [ ] **Weak password requirements** — No complexity enforcement
- [ ] **Missing brute-force protection** — No rate limiting on login
- [ ] **Session fixation** — Session ID not regenerated after login
- [ ] **Insecure session storage** — Sessions in localStorage (XSS vulnerable)
- [ ] **Missing logout** — No session invalidation
- [ ] **Predictable tokens** — Using weak random generators for tokens
- [ ] **Password in URL** — Credentials in query parameters
- [ ] **Missing MFA on critical operations** — No 2FA for sensitive actions

#### 1.3 Sensitive Data Exposure
- [ ] **Hardcoded secrets** — API keys, passwords, tokens in code
- [ ] **Secrets in logs** — Sensitive data written to console/logs
- [ ] **Secrets in error messages** — Stack traces exposing internals
- [ ] **Unencrypted sensitive data** — PII/credentials not encrypted at rest
- [ ] **Weak cryptography** — MD5, SHA1 for passwords, short keys
- [ ] **Missing HTTPS** — HTTP links for sensitive operations
- [ ] **Sensitive data in URLs** — Tokens/IDs in GET parameters
- [ ] **Excessive data exposure** — Returning more fields than needed in API

#### 1.4 XML External Entities (XXE)
- [ ] **XML parsing without disabling DTD** — External entity processing enabled
- [ ] **Unsafe XML deserialization** — User-controlled XML parsed

#### 1.5 Broken Access Control
- [ ] **Missing authorization checks** — Actions without permission verification
- [ ] **IDOR (Insecure Direct Object Reference)** — Accessing resources by ID without ownership check
- [ ] **Privilege escalation** — User can access admin functions
- [ ] **CORS misconfiguration** — Wildcard or overly permissive origins
- [ ] **Missing function-level access control** — API endpoints without role checks
- [ ] **Path traversal** — User input in file paths (../)
- [ ] **Forced browsing** — Unprotected admin/debug endpoints

#### 1.6 Security Misconfiguration
- [ ] **Debug mode in production** — Verbose errors, stack traces exposed
- [ ] **Default credentials** — Unchanged default passwords
- [ ] **Unnecessary features enabled** — Unused endpoints, methods
- [ ] **Missing security headers** — No CSP, X-Frame-Options, etc.
- [ ] **Directory listing enabled** — Exposed file structure
- [ ] **Outdated dependencies** — Known vulnerabilities in packages
- [ ] **Permissive file permissions** — World-readable sensitive files

#### 1.7 Cross-Site Scripting (XSS)
- [ ] **Reflected XSS** — User input echoed without encoding
- [ ] **Stored XSS** — Database content rendered without sanitization
- [ ] **DOM XSS** — Client-side JS using unsafe sinks (innerHTML, eval)
- [ ] **Missing Content-Security-Policy** — No CSP headers
- [ ] **Unsafe React patterns** — dangerouslySetInnerHTML with user content
- [ ] **Template literal injection** — User input in template strings

#### 1.8 Insecure Deserialization
- [ ] **Unsafe JSON parsing** — eval() for JSON
- [ ] **Object deserialization** — pickle, serialize without validation
- [ ] **Prototype pollution** — Object.assign/merge with user input

#### 1.9 Using Components with Known Vulnerabilities
- [ ] **Outdated dependen