Cursor rule

1---
2description: "AI-powered security analysis of code changes — traces data flow, detects injection, auth bypass, secrets exposure, and unsafe deserialization across files. Use when reviewing pending changes, before release-branch, during verify-work Phase 5, during build-epic Step 0 threat modeling, or when the user says \"security review\" or \"scan for vulns\"."
3alwaysApply: false
4---
5 
6# story: e45s41
7<!-- story: e45s18 -->
8# story: e26s01
9# story: e26s02
10# story: e26s03
11# story: e26s04
12# story: e26s05
13# story: e26s06
14# story: e26s07
15# story: e45s26
16 
17# Security Review
18 
19> **HARD GATE** — Requires git context (branch with merge-base or diff). Never
20> writes files outside `specs/security/`. Findings below confidence 8/10 are
21> suppressed. Pre-flight: `git rev-parse HEAD >/dev/null 2>&1`
22 
23## Parallel worktree mode (e45s18)
24 
25When running alongside `audit-code`, use isolated worktrees so scans do not race on the same index:
26 
27```bash
28bash scripts/lib/parallel-review-worktrees.sh security-review
29```
30 
31Each check gets a detached worktree at `.bigpowers/worktrees/review-<name>/`; reports still write only under `specs/security/`.
32 
33## 5-phase scan
34 
35| # | Phase | What |
36|---|-------|------|
37| 1 | **Scope Resolution** | Detect diff via `git diff --merge-base origin/HEAD`; resolve languages/frameworks from dependency files |
38| 2 | **Context Research** | Identify existing security patterns, sanitization, auth model in the codebase |
39| 3 | **Vulnerability Assessment** | Trace user input → sink; check auth boundaries, crypto, deserialization, path ops |
40| 4 | **False-Positive Filtering** | Cross-check each finding against exclusion rules; reject confidence < 8 |
41| 5 | **Report Generation** | Output structured markdown: file:line, severity, category, exploit scenario, fix |
42 
43## Categories
44 
45Covered: SQLi, XSS, SSRF, command injection, auth bypass, unsafe deserialization, path traversal, IDOR, crypto flaws, secrets exposure, template injection, NoSQLi
46 
47## CWE mapping mandate (e45s26)
48 
49Every **new detection rule** added to this skill MUST:
50 
511. Map to a [CWE](https://cwe.mitre.org/) ID in `REFERENCE-vuln-categories.md` (e.g. SQLi → CWE-89, XSS → CWE-79).
522. Ship **two fixture pairs** under `skills/security-review/fixtures/`:
53   - **Positive** — minimal code the rule MUST flag (vulnerable pattern present).
54   - **Negative** — structurally similar code the rule MUST NOT flag (safe pattern / false-positive guard).
55 
56| Rule | CWE | Positive fixture | Negative fixture |
57|------|-----|------------------|------------------|
58| SQL injection | CWE-89 | `fixtures/CWE-89-sqli-positive.py` | `fixtures/CWE-89-sqli-negative.py` |
59| XSS (DOM) | CWE-79 | `fixtures/CWE-79-xss-positive.js` | `fixtures/CWE-79-xss-negative.js` |
60| Missing tenant scoping (IDOR) | CWE-639 | `fixtures/CWE-639-idor-positive.go` | `fixtures/CWE-639-idor-negative.go` |
61| Fail-open verify directive | CWE-754 | `fixtures/CWE-fail-open-verify-positive.sh` | `fixtures/CWE-fail-open-verify-negative.sh` |
62 
63Before merging a new category, run both fixtures through the detection guidance and confirm positive flags / negative passes.
64 
65## SQL-safety doctrine (e45s41 — proven authorship)
66 
67Formal rule for SQL injection classification:
68 
69| SQL source | Attacker-reachable input? | Verdict |
70|------------|---------------------------|---------|
71| Hardcoded / compile-time constant string | N/A | **Safe** — proven authorship |
72| Developer-authored query with bound parameters only | No dynamic fragments from user input | **Safe** |
73| String concatenation / template with user-controlled values | Yes | **Unsafe** — report as SQLi |
74| ORM query builder with user input in WHERE/JOIN | Yes | **Unsafe** unless parameterized |
75| Stored procedure call with bound args | Args from trusted constants only | **Safe** |
76| Stored procedure with dynamic SQL inside | User input reaches EXEC | **Unsafe** |
77 
78**Provenance test:** If the agent cannot prove the query string was authored entirely by the developer (no attacker-reachable interpolation), treat as vulnerable. Hardcoded SQL in migrations, seeds, and admin scripts is safe; anything reachable from HTTP/CLI/user input is not.
79 
80## BCP Plus Integration
81 
82This skill maps to **BCP Plus dimension 12 (Security & Compliance)**. When BCP Plus sizing is active, the threat model categories above correspond to sub-elements within dimension 12. The NFR Gate rule applies: standard-expectation items (e.g., "use HTTPS", "hash passwords") score 0 with a one-line rationale; only above-standard security requirements contribute to the dimension 12 count. See `docs/references/bcp-plus.md` for the full 13-dimension framework and NFR Gate pattern.
83 
84## Integration points
85 
86| Skill | Touchpoint |
87|-------|------------|
88| `build-epic` | Step 0 — threat-model epic scope → `specs/security/epics/<id>/THREAT_MODEL.md` |
89| `plan-work` | `security:` field (none/low/medium/high) on story tasks |
90| `plan-release` | +2 WSJF risk boost for HIGH+ risk epics |
91| `audit-code` | Checklist: "diff scanned — no unaddressed HIGH findings" |
92| `request-review` | Inject threat model categories + false-positive rules into reviewer prompt |
93| `investigate-bug` | Security-impact assessment in RCA (NONE→CRITICAL) |
94| `validate-fix` | Recurrence hardening check for security bugs |
95| `verify-work` | Phase 5 — blocks on HIGH findings ≥ 8 confidence |
96| `release-branch` | Hard gate — blocks merge if unresolved HIGH findings |
97 
98## Report format
99 
100Each finding: **`File:Line` — Severity — Category**
101- Description: how the vulnerability manifests
102- Exploit scenario: concrete attack path
103- Recommendation: fix with code example
104 
105## Reference files
106 
107- [Vuln categories](REFERENCE-vuln-categories.md) — detection guidance per vuln type
108- [False positives](REFERENCE-false-positives.md) — hard exclusions + precedent
109- [Confidence rubric](REFERENCE-confidence-rubric.md) — scoring methodology (0–10)
110 
111## Verify
112 
113→ verify: `test -d specs/security && test -f scripts/lib/parallel-review-worktrees.sh && bash scripts/verify-cwe-fixture-sync.sh >/dev/null && git rev-parse HEAD >/dev/null 2>&1`
114 
115---
116 
117# Confidence Scoring Rubric
118 
119Every finding that survives Phase 4 false-positive filtering receives a confidence
120score from 1 (speculative) to 10 (certain). Only findings ≥ 8 are reported.
121 
122## Score 9–10: Certain Exploit Path
123 
124**Criteria:**
125- Concrete, testable exploit with clear reproduction steps
126- No assumptions about uncommon configurations
127- No chain of multiple unlikely conditions
128- Attacker has full control over the input vector
129 
130**Examples:**
131- User-supplied SQL in a `SELECT` statement with no parameterization
132- `os.system(f"rm {user_path}")` where user controls the path
133- Pickle deserialization of user-supplied data without any wrapping
134 
135**Severity:** HIGH
136 
137## Score 8: Clear Vulnerability Pattern
138 
139**Criteria:**
140- Well-known vulnerability pattern with standard exploitation method
141- Requires specific conditions but conditions are commonly met
142- Exploitability is well-documented in OWASP / CVE databases
143 
144**Examples:**
145- JWT without signature verification in authentication middleware
146- SSRF where attacker controls the full URL including host
147- Hardcoded AWS secret key in source code
148 
149**Severity:** HIGH or MEDIUM
150 
151## Score 7: Suspicious Pattern
152 
153**Criteria:**
154- Unusual code that may indicate a vulnerability
155- Requires specific conditions that may not be present
156- Alternative secure interpretation is equally likely
157- Defense-in-depth concern rather than direct exploit
158 
159**Examples:**
160- A function accepting user input that passes through multiple layers before reaching a sink (unclear if sanitized)
161- Custom encryption implementation (likely weak, but may not process sensitive data)
162- Path construction that looks safe but has a subtle bypass
163 
164**Severity:** LOW or suppress
165 
166## Score < 7: Do Not Report
167 
168**Criteria:**
169- Theoretical concern without exploit path
170- Requires unrealistic attacker capabilities
171- Violates one or more hard exclusion rules
172- Better handled by separate tooling (dependency scanner, SAST, secret scanner)
173- Purely stylistic or best-practice concern without security impact
174 
175**Examples:**
176- "This function doesn't validate all inputs" without proving the validated input is the attack surface
177- "This uses MD5" where the hash is not used for security (e.g., cache key)
178- "This function could consume too much memory" (DOS exclusion)
179 
180**Action:** Suppress entirely. Do not include in report.
181 
182## Severity Mapping
183 
184Once confidence ≥ 8 is confirmed, map to severity:
185 
186| Severity | Impact | Examples |
187|----------|--------|---------|
188| **CRITICAL** | Remote compromise, full data breach | RCE, auth bypass with admin escalation, SQLi with data exfiltration |
189| **HIGH** | Significant security boundary crossed | SSRF to internal services, hardcoded cloud credentials, insecure deserialization |
190| **MEDIUM** | Limited impact or requires conditions | Stored XSS behind auth, IDOR on non-sensitive data, weak but not broken crypto |
191| **LOW** | Defense-in-depth, minimal blast radius | Missing security header, verbose error messages in non-production |
192 
193## Quality Gate
194 
195The confidence rubric double-checks each finding against three lenses:
196 
197| Lens | Question |
198|------|----------|
199| **Exploitability** | Can a real attacker trigger this from a trust boundary? |
200| **Actionability** | Would a security engineer accept a fix recommendation for this? |
201| **Precedent** | Has this type of finding passed/failed human review before? |
202 
203---
204 
205# False-Positive Exclusion Rules
206 
207Applied during Phase 4 of the scan. Findings matching any hard exclusion are
208automatically suppressed. Precedents from prior reviews guide borderline cases.
209 
210## Hard Exclusions
211 
212Automatically exclude findings matching these patterns:
213 
214| # | Rule | Rationale |
215|---|------|-----------|
216| 1 | **Denial of Service (DOS)** — resource exhaustion, CPU/memory attacks | Handled separately; not actionable in code review |
217| 2 | **Secrets on disk** if otherwise secured | Secrets management is a separate concern |
218| 3 | **Rate limiting** concerns | Operational, not a code vulnerability |
219| 4 | **Memory consumption / CPU exhaustion** | Not actionable in diff review |
220| 5 | **Input validation on non-security-critical fields** without proven exploit path | Theoretical, not concrete |
221| 6 | **GitHub Actions input sanitization** unless clearly triggerable via untrusted input | Most workflow vulns are not exploitable |
222| 7 | **Lack of hardening measures** | Code is not expected to implement all best practices |
223| 8 | **Race conditions / timing attacks** that are theoretical | Only report if concretely problematic |
224| 9 | **Outdated third-party libraries** | Managed separately by dependency scanners |
225| 10 | **Memory safety** in Rust or other memory-safe languages | Impossible by language guarantees |
226| 11 | **Hardcoded SQL with proven authorship** — migrations, seeds, static admin queries with no user interpolation | Developer-authored SQL is safe per SQL-safety doctrine (e45s41) |
227| 12 | **Unit test files only** | Not production risk |
228| 13 | **Log spoofing** | Outputting unsanitized input to logs is not a vuln |
229| 14 | **SSRF that only controls path** | Only host/protocol control is exploitable |
230| 15 | **User-controlled content in AI system prompts** | Not a security vulnerability |
231| 16 | **Regex injection** | Injecting untrusted content into regex is not a vuln |
232| 17 | **Regex DOS** | Excluded alongside general DOS |
233| 18 | **Documentation files** (.md, .txt) | Insecure docs are not code vulnerabilities |
234| 19 | **Lack of audit logs** | Not a vulnerability |
235 
236## Precedent Rules
237 
238These guide borderline cases based on prior human review decisions:
239 
240| # | Precedent | Reasoning |
241|---|-----------|-----------|
242| 1 | **Logging high-value secrets in plaintext IS a vuln.** Logging URLs is safe. | Secrets in logs = credential exposure; URLs are not secrets |
243| 2 | **UUIDs are unguessable** — no validation needed | Cryptographic property of UUID v4/v7 |
244| 3 | **Environment variables and CLI flags are trusted values** | Attackers cannot modify these in secure environments |
245| 4 | **Resource management issues** (memory leaks, fd leaks) are NOT valid | Operational, not security |
246| 5 | **Tabnabbing, XS-Leaks, prototype pollution, open redirects** — do NOT report unless extremely high confidence | Subtle, low-impact, high false-positive rate |
247| 6 | **React/Angular XSS** — safe unless `dangerouslySetInnerHTML`, `bypassSecurityTrustHtml`, etc. | Framework auto-escapes |
248| 7 | **GitHub Action workflow vulns** — verify concrete attack path before reporting | Most are theoretical |
249| 8 | **Client-side JS/TS auth checks** — not a vuln; server is authoritative | Client code is untrusted |
250| 9 | **IPython notebook vulns** — only report if concrete untrusted-input trigger | Most are not exploitable |
251| 10 | **Logging non-PII data** — not a vuln even if sensitive. Only PII/secrets/passwords. | Intent: operational logging vs credential exposure |
252| 11 | **Shell script command injection** — only report if concrete untrusted-input path | Most shell scripts don't process untrusted input |
253 
254## Confidence Scoring
255 
256Findings that survive exclusions get a confidence score (1–10):
257 
258| Range | Meaning | Action |
259|-------|---------|--------|
260| 9–10 | Certain exploit path, testable | Report as HIGH |
261| 8 | Clear vulnerability pattern | Report as HIGH/MEDIUM |
262| 7 | Suspicious, needs conditions | Report as LOW or suppress |
263| <7 | Too speculative | **Do not report** |
264 
265**Hard threshold:** Only report findings with confidence ≥ 8.
266 
267## Signal Quality Criteria
268 
269For remaining findings, assess:
2701. Is there a concrete, exploitable vulnerability with a clear attack path?
2712. Does this represent a real security risk (vs theoretical best practice)?
2723. Are there specific code locations and reproduction steps?
2734. Would this finding be actionable for a security team?
274 
275---
276 
277# Vulnerability Categories — Detection Guidance
278 
279Each category: vulnerable pattern → safe pattern → code example.
280 
281## SQL Injection
282 
283| Aspect | Detail |
284|--------|--------|
285| **Vulnerable** | String interpolation in SQL queries: `f"SELECT * FROM users WHERE id = {uid}"` |
286| **CWE** | CWE-89 (SQL Injection) |
287| **Fixtures** | `fixtures/CWE-89-sqli-positive.py` (flag) · `fixtures/CWE-89-sqli-negative.py` (pass) |
288| **Safe** | Parameterized queries / ORM: `cursor.execute("SELECT * FROM users WHERE id = %s", (uid,))` |
289| **Look for** | f-strings, `+` concatenation, `format()` in query builders; raw SQL in ORM `.raw()` / `.execute()` |
290| **False-positive guard** | Not a FP if the input is user-controlled (HTTP param, file, env var, CLI arg). Env vars are trusted (see exclusion rules). |
291 
292## Cross-Site Scripting (XSS)
293 
294| Aspect | Detail |
295|--------|--------|
296| **Vulnerable** | `element.innerHTML = userInput`, `dangerouslySetInnerHTML={{__html: userInput}}` |
297| **CWE** | CWE-79 (Cross-site Scripting) |
298| **Fixtures** | `fixtures/CWE-79-xss-positive.js` (flag) · `fixtures/CWE-79-xss-negative.js` (pass) |
299| **Safe** | `element.textContent = userInput`, React JSX (auto-escaped), template engines with auto-escaping |
300| **Look for** | `.innerHTML`, `document.write()`, `dangerouslySetInnerHTML`, `v-html` (Vue), `bypassSecurityTrustHtml` (Angular) |
301| **False-positive guard** | React/Angular components without unsafe methods are NOT vulnerable (see exclusion rules). |
302 
303## Server-Side Request Forgery (SSRF)
304 
305| Aspect | Detail |
306|--------|--------|
307| **Vulnerable** | User-controlled URL passed to server-side HTTP client: `requests.get(user_url)` |
308| **Safe** | URL allowlist validation, internal-network blocking, protocol/host restriction |
309| **Look for** | User input → `fetch`, `requests.get`, `axios.get`, `urllib`, `curl`, `http.get`; host control only (path-only is excluded) |
310 
311## Command Injection
312 
313| Aspect | Detail |
314|--------|--------|
315| **Vulnerable** | User input in shell commands: `os.system(f"ping {host}")`, `subprocess.run(f"grep {pattern} file", shell=True)` |
316| **Safe** | `subprocess.run(["ping", host])` with arguments as list; `shlex.quote()` |
317| **Look for** | `shell=True`, `os.system`, `os.popen`, `exec()`, `eval()`, `$()`, backticks |
318| **False-positive guard** | Shell scripts without untrusted user input are generally not exploitable. |
319 
320## Authentication/Authorization Bypass
321 
322| Aspect | Detail |
323|--------|--------|
324| **Vulnerable** | Missing auth check on protected endpoint; JWT without signature verification; hardcoded admin tokens |
325| **Safe** | Consistent auth middleware; JWT with `RS256`/`HS256` verification; role-based access control |
326| **Look for** | Routes without auth decorators; `@login_required` / `@require_auth` missing; JWT without `.verify()`; client-side auth checks only |
327 
328## Unsafe Deserialization
329 
330| Aspect | Detail |
331|--------|--------|
332| **Vulnerable** | `pickle.load(user_data)`, `yaml.load(user_input)`, `JSON.parse()` on untrusted tokens, `eval(input())` |
333| **Safe** | `yaml.safe_load()`, `json.loads()` (safe for JSON), `pickle.load(weights_only=True)` (PyTorch), schema validation |
334| **Look for** | `pickle.load`, `yaml.load` (not safe_load), `torch.load(weights_only=False)`, `eval`, `marshal.load`, `node-serialize` |
335 
336## Path Traversal
337 
338| Aspect | Detail |
339|--------|--------|
340| **Vulnerable** | User input in file paths: `open(f"/data/{filename}")`, `path.join(base, user_path)` |
341| **Safe** | Path normalization + prefix check: `os.path.realpath(path).startswith(BASE_DIR)`; allowlist of valid filenames |
342| **Look for** | `open()`, `read_file()`, `os.path.join` with user input; `../` traversal without normalization |
343 
344## Insecure Direct Object Reference (IDOR)
345 
346| Aspect | Detail |
347|--------|--------|
348| **Vulnerable** | API endpoint uses user-supplied ID without ownership check: `GET /api/order/{order_id}` — returns any user's order |
349| **Safe** | Ownership verification: verify `order.user_id == current_user.id` before returning data |
350| **Look for** | CRUD endpoints that accept IDs without authorization; horizontal/vertical privilege checks missing |
351 
352## Weak Cryptography
353 
354| Aspect | Detail |
355|--------|--------|
356| **Vulnerable** | MD5/SHA1 for passwords; ECB mode; hardcoded keys; `random` module (not `secrets`); short key lengths |
357| **Safe** | `bcrypt`/`argon2` for passwords; AES-GCM; `secrets` module; RSA 2048+; proper IV generation |
358| **Look for** | `md5`, `sha1`, `DES`, `ECB`, `PKCS1_v1_5`, `random` for crypto, hardcoded `key=`, `Crypto.Cipher` without AEAD |
359 
360## Secrets Exposure
361 
362| Aspect | Detail |
363|--------|--------|
364| **Vulnerable** | Hardcoded API keys, passwords, tokens in source code; secrets in logs; secrets in client-side code |
365| **Safe** | Environment variables; secret manager (AWS Secrets Manager, HashiCorp Vault); `.env` excluded from VCS |
366| **Look for** | `API_KEY=`, `password=`, `secret=`, `token=` in code; AWS keys, GitHub tokens, Stripe keys, JWTs in source |
367| **False-positive guard** | Secrets stored on disk but otherwise secured ARE excluded. Logging high-value secrets IS a vuln. Logging URLs is safe. |
368 
369## Template Injection (SSTI)
370 
371| Aspect | Detail |
372|--------|--------|
373| **Vulnerable** | User input in template rendering: `Template(user_input).render()`, `render_template_string(user_input)` |
374| **Safe** | Static templates; input passed as context variable, not template string |
375| **Look for** | `render_template_string`, `Template()()` with user string; `eval` in template context; `${user_input}` in JS template literals on server |
376 
377## NoSQL Injection
378 
379| Aspect | Detail |
380|--------|--------|
381| **Vulnerable** | User input in MongoDB queries: `db.users.find({username: user_input})` where input is `{"$gt": ""}` |
382| **Safe** | Schema validation; type checking on query params; ORM sanitization |
383| **Look for** | MongoDB `$where`, `$gt`, `$regex` from user input; raw mongo queries without type coercion |
384