diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 540cf51d..e0874e51 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -45,116 +45,17 @@ jobs: ref: ${{ inputs.ref }} fetch-depth: 0 - - name: Generate diff context + - name: Check diff availability id: diff run: | RANGE="${{ inputs.diff_range }}" if [ -z "$RANGE" ]; then echo "diff_available=false" >> "$GITHUB_OUTPUT" echo "No diff range provided" - exit 0 + else + echo "diff_available=true" >> "$GITHUB_OUTPUT" + echo "Reviewing range: ${RANGE}" fi - echo "diff_available=true" >> "$GITHUB_OUTPUT" - - # File-level diffstat for overview - git diff --stat $RANGE > /tmp/diffstat.txt - # Commit log with full messages - git log --format="- %h %s" $RANGE > /tmp/commits.txt - # Full diff - git diff $RANGE > /tmp/staging-diff.patch - - # Truncate diff if too large (>200KB) to avoid token limits - DIFF_SIZE=$(wc -c < /tmp/staging-diff.patch) - if [ "$DIFF_SIZE" -gt 204800 ]; then - echo "::warning::Diff is ${DIFF_SIZE} bytes, truncating to 200KB" - head -c 204800 /tmp/staging-diff.patch > /tmp/staging-diff-truncated.patch - mv /tmp/staging-diff-truncated.patch /tmp/staging-diff.patch - fi - - # Build the prompt with actual file contents (not heredoc references) - { - cat <<'STATIC_PART' - You are reviewing a batch of commits that just landed on the staging branch - of the IronClaw project (a Rust AI assistant). This is a thorough security - and quality review. - - ## Your Review Process - - 1. **Read CLAUDE.md** for project coding standards and conventions - 2. **Read the full diff** below to understand what changed - 3. **Read the complete source files** for every changed file (use the Read tool) - to understand the full context, not just the diff hunks - 4. **Trace data flows** through changed code paths to find bugs - 5. **Check for security issues** across the OWASP top 10 categories - - ## Severity Categories - - ### CRITICAL - - Security vulnerabilities: injection, XSS, SSRF, path traversal, auth bypass - - Data exfiltration risks: secrets in logs, unescaped user data reaching LLM - - Memory safety: unsafe blocks, unbounded allocations, panics in prod code - - .unwrap() or .expect() in production code (not tests) - - Race conditions in concurrent code (Arc/RwLock misuse, TOCTOU) - - ### HIGH - - Logic bugs that would cause incorrect behavior - - Missing error handling that would cause silent failures - - Breaking changes to public APIs or database schema - - Regression in safety/sanitizer/leak detection layers - - ### MEDIUM - - Missing tests for new functionality - - Unnecessary complexity or poor abstractions - - Performance concerns (N+1 queries, unbounded loops) - - ### LOW - - Documentation gaps, naming suggestions - - ## Confidence Scoring - - For EVERY finding, assign a confidence score from 0 to 100: - - **90-100**: You are certain this is a real issue. You can point to the exact code and explain why it's wrong. - - **70-89**: Very likely a real issue but you'd want a human to verify. - - **50-69**: Possibly an issue but you're not fully sure of the context. - - **0-49**: Speculative. Might be a false positive. - - Be honest with your confidence. A CRITICAL finding you're unsure about should get a low confidence score. - - ## Output Format - - Write your findings to a file called /tmp/review-results.json with this structure: - { - "summary": "One paragraph overall assessment", - "critical": [{"title": "...", "description": "...", "file": "...", "line": 0, "confidence": 85}], - "high": [{"title": "...", "description": "...", "file": "...", "line": 0, "confidence": 75}], - "medium": [{"title": "...", "description": "...", "file": "...", "line": 0, "confidence": 60}], - "low": [{"title": "...", "description": "...", "file": "...", "line": 0, "confidence": 50}] - } - - If you find no issues at a given severity, use an empty array. - - ## Context - - Changed files: - STATIC_PART - cat /tmp/diffstat.txt - echo "" - echo "Commits:" - cat /tmp/commits.txt - echo "" - echo "Diff:" - cat /tmp/staging-diff.patch - } > /tmp/review-prompt.txt - - - name: Load prompt into env - id: load-prompt - if: steps.diff.outputs.diff_available == 'true' - run: | - # Use GITHUB_ENV with delimiter to handle multiline content - DELIM=$(openssl rand -hex 16) - echo "REVIEW_PROMPT<<${DELIM}" >> "$GITHUB_ENV" - cat /tmp/review-prompt.txt >> "$GITHUB_ENV" - echo "${DELIM}" >> "$GITHUB_ENV" - name: Run Claude Code review id: claude-review @@ -162,22 +63,52 @@ jobs: uses: anthropics/claude-code-action@v1 with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} - prompt: ${{ env.REVIEW_PROMPT }} - claude_args: "--max-turns 30" + prompt: | + Review the diff for range `${{ inputs.diff_range }}` in this repo. + Run `git diff ${{ inputs.diff_range }}` to see what changed. - - name: Validate and process results + Categorize each finding by severity (CRITICAL/HIGH/MEDIUM/LOW) and assign + a confidence score (0-100). Only report issues you're confident about. + + CRITICAL: security vulns, panics in prod (.unwrap/.expect), data exfiltration, race conditions + HIGH: logic bugs, missing error handling, breaking API/schema changes + MEDIUM: missing tests, unnecessary complexity, performance issues + LOW: documentation gaps, naming suggestions + + Confidence: 90-100 = certain, 70-89 = very likely, 50-69 = possible, 0-49 = speculative + + Return structured JSON with: summary, critical[], high[], medium[], low[] + Each finding: {title, description, file, line, confidence} + claude_args: "--max-turns 5" + + - name: Extract results from execution log id: process-results if: always() && steps.diff.outputs.diff_available == 'true' run: | - # Default: no results - if [ ! -f /tmp/review-results.json ]; then - echo '{"summary":"Claude review did not produce structured output.","critical":[],"high":[],"medium":[],"low":[]}' > /tmp/review-results.json - fi + FALLBACK='{"summary":"Claude review did not produce structured output.","critical":[],"high":[],"medium":[],"low":[]}' + EXEC_FILE="${{ steps.claude-review.outputs.execution_file }}" - # Validate JSON - if ! jq empty /tmp/review-results.json 2>/dev/null; then - echo "::warning::review-results.json is not valid JSON, using fallback" - echo '{"summary":"Claude review produced invalid JSON.","critical":[],"high":[],"medium":[],"low":[]}' > /tmp/review-results.json + if [ -n "$EXEC_FILE" ] && [ -f "$EXEC_FILE" ]; then + # Extract the last assistant text message as the review result + RESULT=$(jq -r '[.[] | select(.type == "assistant" and .message != null) | .message] | last // empty' "$EXEC_FILE" 2>/dev/null || true) + + # Try to parse as JSON (Claude may wrap in markdown code block) + if [ -n "$RESULT" ]; then + # Strip markdown code fences if present + CLEANED=$(echo "$RESULT" | sed 's/^```json//;s/^```//;s/```$//' | jq '.' 2>/dev/null || true) + if [ -n "$CLEANED" ]; then + echo "$CLEANED" > /tmp/review-results.json + else + echo "::warning::Could not parse Claude output as JSON, using fallback" + echo "$FALLBACK" > /tmp/review-results.json + fi + else + echo "::warning::No assistant messages in execution log, using fallback" + echo "$FALLBACK" > /tmp/review-results.json + fi + else + echo "::warning::No execution file found, using fallback" + echo "$FALLBACK" > /tmp/review-results.json fi # Ensure confidence field exists on all findings (default to 50 if missing)