bmad-security-review

name	bmad-security-review
description	Manual 3-persona adversarial security review for PRs — Blind Hunter, Edge Case Hunter, Acceptance Auditor. Use when Pi/ZAI are unreliable for analytical tasks.

BMAD Security Review — Manual 3-Persona Adversarial Analysis

What It Does

Runs a structured adversarial security review on code changes (PRs, feature branches) using 3 security personas. Designed as a manual alternative to Pi/ZAI-based party mode when those tools are rate-limited, unreliable, or unavailable for analytical tasks.

When to Use

• After implementing security-sensitive features (auth, RLS, RBAC, public APIs)
• Before merging PRs that touch authentication, authorization, or data isolation
• When Pi/ZAI are rate-limited (429) or produce unreliable analysis
• As a faster alternative to full 6-persona BMAD party mode for code-level reviews

The 3 Personas

1. 🔴 Blind Hunter (Attacker Perspective)

Role: Assumes zero context — reads code as an attacker would, looking for exploitable patterns.

Focus areas:

• Unprotected routes or endpoints (dead code that was never wired)
• Missing middleware (auth, rate limiting, CORS)
• SQL injection, wildcard injection (ILIKE %, _)
• IDOR (Insecure Direct Object Reference) — can user A access user B's data?
• Broken access control — can lower roles access higher-role endpoints?
• Information leakage in error responses

Output format:

[Finding ID] [Severity: CRITICAL/HIGH/MEDIUM/LOW] Short title
File: path/to/file.go:line
Description: What the vulnerability is
Attack vector: How an attacker would exploit it

2. 🟠 Edge Case Hunter (Chaos Engineer)

Role: Thinks about unusual scenarios, race conditions, and boundary cases that normal review misses.

Focus areas:

• Race conditions (Find → Mutate patterns, TOCTOU)
• SET LOCAL failure in RLS middleware (silent failure = data leakage)
• N+1 queries under load
• Slug validation (format, length, special characters)
• Concurrent operations (double-spend, duplicate creates, stale reads)
• Error handling gaps (what happens when DB returns unexpected errors?)
• Empty/nil inputs, overflow values, extremely long strings

Output format:

[Finding ID] [Severity: CRITICAL/HIGH/MEDIUM/LOW] Short title
File: path/to/file.go:line
Description: What happens in the edge case
Trigger: Conditions needed to trigger
Impact: What goes wrong

3. 🟢 Acceptance Auditor (QA Reviewer)

Role: Verifies the implementation matches the specification/architecture doc. Checks code quality and consistency.

Focus areas:

• Router wiring completeness (all handler functions registered)
• Response format consistency (JSONError vs raw http.Error)
• Custom utility functions that should use stdlib (itoa → strconv.Itoa)
• Test coverage gaps (untested edge cases, missing unit tests)
• Code style consistency with existing codebase
• Dead code or unused functions

Output format:

[Finding ID] [Severity: CRITICAL/HIGH/MEDIUM/LOW] Short title
File: path/to/file.go:line
Description: What's inconsistent or missing
Expected: What the spec/codebase convention requires

Workflow

Step 1: Gather Context

Read all changed files in the PR/branch. For BOND specifically:

git diff develop...feature-branch --stat   # List changed files
git diff develop...feature-branch         # Full diff

Also read: docs/ARCHITECTURE.md for architecture decisions, AGENT.md for project conventions, and any existing security patterns in the codebase.

Step 2: Run Each Persona

Go through each file systematically with each persona's lens. Read handler → repository → service → test files in order.

• Follow the request flow: HTTP handler → middleware → service → repository → SQL
• Check BOTH the happy path AND error paths
• Look at what's NOT there (missing routes, missing validation)
• Cross-reference router wiring in main.go with handler methods

Step 3: Consolidate Findings

Combine all findings, deduplicate, assign severity:

• CRITICAL — Exploitable now, data breach or auth bypass → Must fix before merge
• HIGH — Exploitable with specific conditions → Must fix before merge
• MEDIUM — Security hardening, defense in depth → Should fix before merge
• LOW — Code quality, consistency → Nice to have

Step 4: Fix and Verify

Fix all findings, then verify:

# Go validation
cd ${PROJECT_DIR} && export PATH="${GO_SDK_PATH}:$PATH" CGO_ENABLED=0
go vet ./bond/internal/...
go test ./bond/internal/... -count=1

# Frontend (if applicable)
cd ${PROJECT_DIR}/web && npx svelte-check --threshold error
npm run build

Output Template

## 🔒 BMAD Security Review — [PR/Feature Name]

### Scope
- Files reviewed: N files | Lines changed: +XXX -XX

### Findings
| ID | Severity | Persona | Summary | File |
|----|----------|---------|---------|------|
| F1 | CRITICAL | Blind Hunter | [Title] | path:line |

### Fixes Applied
| ID | Fix Description |
|----|----------------|
| F1 | [What was changed] |

### Verification
- ✅ `go vet ./...` — clean | ✅ `go test ./...` — all pass | ✅ CI green

Common Finding Patterns

See references/finding-patterns.md for detailed descriptions of frequently found issues: dead code, missing middleware, ILIKE wildcard injection, silent SET LOCAL failure, N+1 queries, custom utils replacing stdlib, and missing slug validation.

Relationship to bmad-party-mode-workflow

	bmad-party-mode-workflow	bmad-security-review
Purpose	Product/business decisions	Code-level security review
Personas	6 (Winston, Amelia, Mary, John, Paige, Tesla)	3 (Blind Hunter, Edge Case Hunter, Acceptance Auditor)
Input	Research data, market analysis	Code diff, architecture docs
Output	Strategic recommendation + consensus	Findings list with severity + fixes
Tool dependency	Pi/ZAI preferred	Manual (no tool dependency)

Use both: bmad-party-mode-workflow for strategic decisions, bmad-security-review for code security.