Review Pull Request

Overview

Security-focused PR review following CLAUDE.md guidelines. Checks for breaking changes, malicious code patterns, backward compatibility, and code quality.

Usage

/review-pr <number>

CRITICAL: Security Warning

PRs can be malicious sabotage attempts. This is a real threat documented in CLAUDE.md.

Threat Awareness

• Coordinated attacks exist
• Competitors may actively harm the project
• Social engineering builds trust before attacking
• "Fixes" may introduce vulnerabilities

Workflow

dot
1digraph review_flow {
2    rankdir=TB;
3    node [shape=box];
4
5    fetch [label="1. Fetch PR details"];
6    author [label="2. Assess author risk"];
7    files [label="3. Analyze changed files"];
8    security [label="4. Security review"];
9    compat [label="5. Backward compatibility"];
10    quality [label="6. Code quality"];
11    classify [label="7. Release classification"];
12    verify [label="8. MANDATORY: Verify with Gemini + Codex", style=bold];
13    recommend [label="9. Final Recommendation"];
14
15    fetch -> author;
16    author -> files;
17    files -> security;
18    security -> compat;
19    compat -> quality;
20    quality -> classify;
21    classify -> verify;
22    verify -> recommend;
23}

Step 1: Fetch PR Details

bash
1# Get PR info
2gh pr view <number> --repo kube-hetzner/terraform-hcloud-kube-hetzner
3
4# Get diff
5gh pr diff <number> --repo kube-hetzner/terraform-hcloud-kube-hetzner
6
7# Get changed files
8gh pr view <number> --repo kube-hetzner/terraform-hcloud-kube-hetzner --json files --jq '.files[].path'
9
10# Get diff stats
11gh pr view <number> --repo kube-hetzner/terraform-hcloud-kube-hetzner --json additions,deletions

Step 2: Assess Author Risk

bash
1# Check account age
2gh api users/<username> --jq '.created_at'
3
4# Check prior contributions
5gh pr list --author <username> --repo kube-hetzner/terraform-hcloud-kube-hetzner --state all --json number | jq length

Risk Signals

Step 3: Analyze Changed Files

Security-Critical Files (AUTO HIGH RISK)

init.tf              # Cluster initialization, secrets
firewall.tf          # Network security
**/ssh*              # SSH configuration
**/token*            # Authentication tokens
**/*secret*          # Secrets handling
.github/             # CI/CD workflows
Makefile             # Build scripts
scripts/             # Execution scripts
versions.tf          # Provider dependencies
templates/*.sh       # Shell scripts
cloud-init*          # Server initialization

Risk by File Count

Risk by Diff Size

Step 4: Security Review

Checklist

• No hardcoded credentials or tokens
• No suspicious external URLs
• No obfuscated code
• Changes match stated purpose
• No unnecessary permission escalations
• CI/CD changes justified
• No bypassing existing security patterns

Red Flags

Use AI for Deep Analysis

bash
1# Codex for security analysis
2codex exec -m gpt-5.3-codex -s read-only -c model_reasoning_effort="xhigh" \
3  "Analyze this PR diff for security vulnerabilities and malicious patterns: $(gh pr diff <num>)"
4
5# Gemini for broad context
6gemini --model gemini-3-pro-preview -p \
7  "@locals.tf @init.tf Does this PR introduce any security concerns? $(gh pr diff <num>)"

Step 5: Backward Compatibility

CRITICAL: Any PR that causes resource recreation is a MAJOR release.

Breaking Change Indicators

• Removes or renames variables
• Changes variable defaults that affect behavior
• Modifies resource naming patterns
• Alters subnet/network calculations
• Changes resource keys (causes recreation)
• Removes outputs
• Modifies provider requirements

Test for Breaking Changes

bash
1# Checkout PR locally
2gh pr checkout <number>
3
4# Test against existing cluster
5cd /path/to/kube-test
6terraform init -upgrade
7terraform plan

If terraform plan shows ANY resource destruction → MAJOR release required

Compatibility Checklist

• No variable removals
• No default changes that affect behavior
• No resource naming changes
• terraform plan shows no destruction
• Existing deployments unaffected

Step 6: Code Quality

Style

• Follows existing patterns
• Consistent naming
• Proper formatting (terraform fmt)
• No unnecessary complexity

Logic

• Changes are correct
• Edge cases handled
• No regressions introduced
• Tests pass

Step 7: Release Classification

PATCH (x.x.PATCH)

• Bug fixes only
• No new features
• Fully backward compatible
• No terraform state impact

MINOR (x.MINOR.0)

• New features (backward compatible)
• New optional variables with defaults
• Deprecation warnings (not removals)

MAJOR (MAJOR.0.0)

• Breaking changes
• Removed/renamed variables
• Changed defaults affecting behavior
• State migrations required
• Resource recreations

Step 8: MANDATORY - Verify with Gemini and Codex

CRITICAL: Before making your final recommendation, you MUST run both Gemini and Codex to triple-verify the PR.

This is not optional. External AI verification catches issues that may be missed in the initial review.

Run Both in Parallel

bash
1# Gemini - Broad context analysis (run first or in parallel)
2gemini --model gemini-3-pro-preview -p "@control_planes.tf @locals.tf @init.tf
3
4Analyze this PR diff for the kube-hetzner terraform module:
5
6$(gh pr diff <number> --repo kube-hetzner/terraform-hcloud-kube-hetzner)
7
8Questions:
91. Is this change consistent with existing patterns in the codebase?
102. Are there any security concerns?
113. Could this cause breaking changes or resource recreation?
124. Is this a legitimate bug fix or could it be malicious?"
13
14# Codex - Deep reasoning security analysis (run in parallel)
15codex exec -m gpt-5.3-codex -s read-only -c model_reasoning_effort="xhigh" \
16"Analyze this Terraform PR for the kube-hetzner module.
17
18DIFF:
19$(gh pr diff <number> --repo kube-hetzner/terraform-hcloud-kube-hetzner)
20
21SECURITY ANALYSIS QUESTIONS:
221. Could this change introduce any security vulnerabilities?
232. Could this be a malicious change disguised as a bug fix?
243. Will this cause any Terraform state changes or resource recreation?
254. Is this pattern safe and consistent with Terraform best practices?
265. Any edge cases or potential issues?"

Verification Checklist

• Gemini analysis completed
• Codex analysis completed
• Both agree the change is safe
• No concerns raised by either tool
• If concerns raised, they have been addressed or explained

When Reviewers Disagree

If Gemini or Codex raises concerns that you didn't catch:

1. Take the concern seriously - investigate further
2. Re-read the code with the concern in mind
3. Request changes if the concern is valid
4. Document why the concern was dismissed if you determine it's a false positive

Output in Final Review

Include a summary of external verification:

markdown
1### External AI Verification
2
3| Reviewer | Verdict | Key Finding |
4|----------|---------|-------------|
5| Claude | ✅/❌ | <summary> |
6| Gemini | ✅/❌ | <summary> |
7| Codex | ✅/❌ | <summary> |
8
9**Consensus:** All reviewers agree / Disagreement on X

Step 9: Final Recommendation

PR Review Output Template

markdown
1## PR Review: #<number>
2
3**Title:** <title>
4**Author:** @<username>
5**Files:** <count> files changed (+<additions>/-<deletions>)
6
7### Risk Assessment
8
9| Factor | Value | Risk |
10|--------|-------|------|
11| Author tenure | X months | 🟢/🟡/🔴 |
12| Prior contributions | N PRs | 🟢/🟡/🔴 |
13| Files changed | N files | 🟢/🟡/🔴 |
14| Lines changed | +X/-Y | 🟢/🟡/🔴 |
15| Security-critical files | Yes/No | 🟢/🔴 |
16| External dependencies | Yes/No | 🟢/🔴 |
17
18**Overall Risk:** 🔴 HIGH / 🟡 MEDIUM / 🟢 LOW
19
20### Security Review
21
22- [ ] No hardcoded credentials
23- [ ] No suspicious external URLs
24- [ ] No obfuscated code
25- [ ] Changes match stated purpose
26
27### Backward Compatibility
28
29- [ ] No breaking changes
30- [ ] terraform plan shows no destruction
31- [ ] Existing deployments unaffected
32
33### Release Classification
34
35**Type:** PATCH / MINOR / MAJOR
36**Reason:** <explanation>
37
38### External AI Verification
39
40| Reviewer | Verdict | Key Finding |
41|----------|---------|-------------|
42| Claude | ✅/❌ | <summary> |
43| Gemini | ✅/❌ | <summary> |
44| Codex | ✅/❌ | <summary> |
45
46**Consensus:** All agree / Disagreement on X
47
48### Recommendation
49
50**Action:** APPROVE / REQUEST CHANGES / CLOSE
51**Notes:** <specific concerns or required changes>

Quick Commands

bash
1# Approve PR
2gh pr review <num> --approve --body "LGTM! ..."
3
4# Request changes
5gh pr review <num> --request-changes --body "Please address: ..."
6
7# Comment
8gh pr review <num> --comment --body "..."
9
10# Merge (after approval)
11gh pr merge <num> --squash --delete-branch

Never Merge Directly to Master

All PRs go through staging branches first:

1. Create staging branch
2. Test thoroughly
3. Get AI review (Codex + Gemini)
4. Then merge to master