security-review

<!-- story: e45s41 --> <!-- story: e45s18 --> <!-- story: e26s01 --> <!-- story: e26s02 --> <!-- story: e26s03 --> <!-- story: e26s04 --> <!-- story: e26s05 --> <!-- story: e26s06 --> <!-- story: e26s07 --> <!-- story: e45s26 -->

Security Review

HARD GATE — Requires git context (branch with merge-base or diff). Never writes files outside specs/security/. Findings below confidence 8/10 are suppressed. Pre-flight: git rev-parse HEAD >/dev/null 2>&1

Parallel worktree mode (e45s18)

When running alongside audit-code, use isolated worktrees so scans do not race on the same index:

bash scripts/lib/parallel-review-worktrees.sh security-review

Each check gets a detached worktree at .bigpowers/worktrees/review-<name>/; reports still write only under specs/security/.

5-phase scan

#	Phase	What
1	Scope Resolution	Detect diff via git diff --merge-base origin/HEAD; resolve languages/frameworks from dependency files
2	Context Research	Identify existing security patterns, sanitization, auth model in the codebase
3	Vulnerability Assessment	Trace user input → sink; check auth boundaries, crypto, deserialization, path ops
4	False-Positive Filtering	Cross-check each finding against exclusion rules; reject confidence < 8
5	Report Generation	Output structured markdown: file:line, severity, category, exploit scenario, fix

Categories

Covered: SQLi, XSS, SSRF, command injection, auth bypass, unsafe deserialization, path traversal, IDOR, crypto flaws, secrets exposure, template injection, NoSQLi

CWE mapping mandate (e45s26)

Every new detection rule added to this skill MUST:

1. Map to a CWE ID in REFERENCE-vuln-categories.md (e.g. SQLi → CWE-89, XSS → CWE-79).
2. Ship two fixture pairs under skills/security-review/fixtures/:
   • Positive — minimal code the rule MUST flag (vulnerable pattern present).
   • Negative — structurally similar code the rule MUST NOT flag (safe pattern / false-positive guard).

Rule	CWE	Positive fixture	Negative fixture
SQL injection	CWE-89	fixtures/CWE-89-sqli-positive.py	fixtures/CWE-89-sqli-negative.py
XSS (DOM)	CWE-79	fixtures/CWE-79-xss-positive.js	fixtures/CWE-79-xss-negative.js
Missing tenant scoping (IDOR)	CWE-639	fixtures/CWE-639-idor-positive.go	fixtures/CWE-639-idor-negative.go
Fail-open verify directive	CWE-754	fixtures/CWE-fail-open-verify-positive.sh	fixtures/CWE-fail-open-verify-negative.sh

Before merging a new category, run both fixtures through the detection guidance and confirm positive flags / negative passes.

SQL-safety doctrine (e45s41 — proven authorship)

Formal rule for SQL injection classification:

SQL source	Attacker-reachable input?	Verdict
Hardcoded / compile-time constant string	N/A	Safe — proven authorship
Developer-authored query with bound parameters only	No dynamic fragments from user input	Safe
String concatenation / template with user-controlled values	Yes	Unsafe — report as SQLi
ORM query builder with user input in WHERE/JOIN	Yes	Unsafe unless parameterized
Stored procedure call with bound args	Args from trusted constants only	Safe
Stored procedure with dynamic SQL inside	User input reaches EXEC	Unsafe

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.

BCP Plus Integration

This 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.

Integration points

Skill	Touchpoint
build-epic	Step 0 — threat-model epic scope → specs/security/epics/<id>/THREAT_MODEL.md
plan-work	security: field (none/low/medium/high) on story tasks
plan-release	+2 WSJF risk boost for HIGH+ risk epics
audit-code	Checklist: "diff scanned — no unaddressed HIGH findings"
request-review	Inject threat model categories + false-positive rules into reviewer prompt
investigate-bug	Security-impact assessment in RCA (NONE→CRITICAL)
validate-fix	Recurrence hardening check for security bugs
verify-work	Phase 5 — blocks on HIGH findings ≥ 8 confidence
release-branch	Hard gate — blocks merge if unresolved HIGH findings

Report format

Each finding: File:Line — Severity — Category

• Description: how the vulnerability manifests
• Exploit scenario: concrete attack path
• Recommendation: fix with code example

Reference files

• Vuln categories — detection guidance per vuln type
• False positives — hard exclusions + precedent
• Confidence rubric — scoring methodology (0–10)

Verify

→ 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

---

Confidence Scoring Rubric

Every finding that survives Phase 4 false-positive filtering receives a confidence score from 1 (speculative) to 10 (certain). Only findings ≥ 8 are reported.

Score 9–10: Certain Exploit Path

Criteria:

• Concrete, testable exploit with clear reproduction steps
• No assumptions about uncommon configurations
• No chain of multiple unlikely conditions
• Attacker has full control over the input vector

Examples:

• User-supplied SQL in a SELECT statement with no parameterization
• os.system(f"rm {user_path}") where user controls the path
• Pickle deserialization of user-supplied data without any wrapping

Severity: HIGH

Score 8: Clear Vulnerability Pattern

Criteria:

• Well-known vulnerability pattern with standard exploitation method
• Requires specific conditions but conditions are commonly met
• Exploitability is well-documented in OWASP / CVE databases

Examples:

• JWT without signature verification in authentication middleware
• SSRF where attacker controls the full URL including host
• Hardcoded AWS secret key in source code

Severity: HIGH or MEDIUM

Score 7: Suspicious Pattern

Criteria:

• Unusual code that may indicate a vulnerability
• Requires specific conditions that may not be present
• Alternative secure interpretation is equally likely
• Defense-in-depth concern rather than direct exploit

Examples:

• A function accepting user input that passes through multiple layers before reaching a sink (unclear if sanitized)
• Custom encryption implementation (likely weak, but may not process sensitive data)
• Path construction that looks safe but has a subtle bypass

Severity: LOW or suppress

Score < 7: Do Not Report

Criteria:

• Theoretical concern without exploit path
• Requires unrealistic attacker capabilities
• Violates one or more hard exclusion rules
• Better handled by separate tooling (dependency scanner, SAST, secret scanner)
• Purely stylistic or best-practice concern without security impact

Examples:

• "This function doesn't validate all inputs" without proving the validated input is the attack surface
• "This uses MD5" where the hash is not used for security (e.g., cache key)
• "This function could consume too much memory" (DOS exclusion)

Action: Suppress entirely. Do not include in report.

Severity Mapping

Once confidence ≥ 8 is confirmed, map to severity:

Severity	Impact	Examples
CRITICAL	Remote compromise, full data breach	RCE, auth bypass with admin escalation, SQLi with data exfiltration
HIGH	Significant security boundary crossed	SSRF to internal services, hardcoded cloud credentials, insecure deserialization
MEDIUM	Limited impact or requires conditions	Stored XSS behind auth, IDOR on non-sensitive data, weak but not broken crypto
LOW	Defense-in-depth, minimal blast radius	Missing security header, verbose error messages in non-production

Quality Gate

The confidence rubric double-checks each finding against three lenses:

Lens	Question
Exploitability	Can a real attacker trigger this from a trust boundary?
Actionability	Would a security engineer accept a fix recommendation for this?
Precedent	Has this type of finding passed/failed human review before?

---

False-Positive Exclusion Rules

Applied during Phase 4 of the scan. Findings matching any hard exclusion are automatically suppressed. Precedents from prior reviews guide borderline cases.

Hard Exclusions

Automatically exclude findings matching these patterns:

#	Rule	Rationale
1	Denial of Service (DOS) — resource exhaustion, CPU/memory attacks	Handled separately; not actionable in code review
2	Secrets on disk if otherwise secured	Secrets management is a separate concern
3	Rate limiting concerns	Operational, not a code vulnerability
4	Memory consumption / CPU exhaustion	Not actionable in diff review
5	Input validation on non-security-critical fields without proven exploit path	Theoretical, not concrete
6	GitHub Actions input sanitization unless clearly triggerable via untrusted input	Most workflow vulns are not exploitable
7	Lack of hardening measures	Code is not expected to implement all best practices
8	Race conditions / timing attacks that are theoretical	Only report if concretely problematic
9	Outdated third-party libraries	Managed separately by dependency scanners
10	Memory safety in Rust or other memory-safe languages	Impossible by language guarantees
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)
12	Unit test files only	Not production risk
13	Log spoofing	Outputting unsanitized input to logs is not a vuln
14	SSRF that only controls path	Only host/protocol control is exploitable
15	User-controlled content in AI system prompts	Not a security vulnerability
16	Regex injection	Injecting untrusted content into regex is not a vuln
17	Regex DOS	Excluded alongside general DOS
18	Documentation files (.md, .txt)	Insecure docs are not code vulnerabilities
19	Lack of audit logs	Not a vulnerability

Precedent Rules

These guide borderline cases based on prior human review decisions:

#	Precedent	Reasoning
1	Logging high-value secrets in plaintext IS a vuln. Logging URLs is safe.	Secrets in logs = credential exposure; URLs are not secrets
2	UUIDs are unguessable — no validation needed	Cryptographic property of UUID v4/v7
3	Environment variables and CLI flags are trusted values	Attackers cannot modify these in secure environments
4	Resource management issues (memory leaks, fd leaks) are NOT valid	Operational, not security
5	Tabnabbing, XS-Leaks, prototype pollution, open redirects — do NOT report unless extremely high confidence	Subtle, low-impact, high false-positive rate
6	React/Angular XSS — safe unless dangerouslySetInnerHTML, bypassSecurityTrustHtml, etc.	Framework auto-escapes
7	GitHub Action workflow vulns — verify concrete attack path before reporting	Most are theoretical
8	Client-side JS/TS auth checks — not a vuln; server is authoritative	Client code is untrusted
9	IPython notebook vulns — only report if concrete untrusted-input trigger	Most are not exploitable
10	Logging non-PII data — not a vuln even if sensitive. Only PII/secrets/passwords.	Intent: operational logging vs credential exposure
11	Shell script command injection — only report if concrete untrusted-input path	Most shell scripts don't process untrusted input

Confidence Scoring

Findings that survive exclusions get a confidence score (1–10):

Range	Meaning	Action
9–10	Certain exploit path, testable	Report as HIGH
8	Clear vulnerability pattern	Report as HIGH/MEDIUM
7	Suspicious, needs conditions	Report as LOW or suppress
<7	Too speculative	Do not report

Hard threshold: Only report findings with confidence ≥ 8.

Signal Quality Criteria

For remaining findings, assess:

1. Is there a concrete, exploitable vulnerability with a clear attack path?
2. Does this represent a real security risk (vs theoretical best practice)?
3. Are there specific code locations and reproduction steps?
4. Would this finding be actionable for a security team?

---

Vulnerability Categories — Detection Guidance

Each category: vulnerable pattern → safe pattern → code example.

SQL Injection

Aspect	Detail
Vulnerable	String interpolation in SQL queries: f"SELECT * FROM users WHERE id = {uid}"
CWE	CWE-89 (SQL Injection)
Fixtures	fixtures/CWE-89-sqli-positive.py (flag) · fixtures/CWE-89-sqli-negative.py (pass)
Safe	Parameterized queries / ORM: cursor.execute("SELECT * FROM users WHERE id = %s", (uid,))
Look for	f-strings, + concatenation, format() in query builders; raw SQL in ORM .raw() / .execute()
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).

Cross-Site Scripting (XSS)

Aspect	Detail
Vulnerable	element.innerHTML = userInput, dangerouslySetInnerHTML={{__html: userInput}}
CWE	CWE-79 (Cross-site Scripting)
Fixtures	fixtures/CWE-79-xss-positive.js (flag) · fixtures/CWE-79-xss-negative.js (pass)
Safe	element.textContent = userInput, React JSX (auto-escaped), template engines with auto-escaping
Look for	.innerHTML, document.write(), dangerouslySetInnerHTML, v-html (Vue), bypassSecurityTrustHtml (Angular)
False-positive guard	React/Angular components without unsafe methods are NOT vulnerable (see exclusion rules).

Server-Side Request Forgery (SSRF)

Aspect	Detail
Vulnerable	User-controlled URL passed to server-side HTTP client: requests.get(user_url)
Safe	URL allowlist validation, internal-network blocking, protocol/host restriction
Look for	User input → fetch, requests.get, axios.get, urllib, curl, http.get; host control only (path-only is excluded)

Command Injection

Aspect	Detail
Vulnerable	User input in shell commands: os.system(f"ping {host}"), subprocess.run(f"grep {pattern} file", shell=True)
Safe	subprocess.run(["ping", host]) with arguments as list; shlex.quote()
Look for	shell=True, os.system, os.popen, exec(), eval(), $(), backticks
False-positive guard	Shell scripts without untrusted user input are generally not exploitable.

Authentication/Authorization Bypass

Aspect	Detail
Vulnerable	Missing auth check on protected endpoint; JWT without signature verification; hardcoded admin tokens
Safe	Consistent auth middleware; JWT with RS256/HS256 verification; role-based access control
Look for	Routes without auth decorators; @login_required / @require_auth missing; JWT without .verify(); client-side auth checks only

Unsafe Deserialization

Aspect	Detail
Vulnerable	pickle.load(user_data), yaml.load(user_input), JSON.parse() on untrusted tokens, eval(input())
Safe	yaml.safe_load(), json.loads() (safe for JSON), pickle.load(weights_only=True) (PyTorch), schema validation
Look for	pickle.load, yaml.load (not safe_load), torch.load(weights_only=False), eval, marshal.load, node-serialize

Path Traversal

Aspect	Detail
Vulnerable	User input in file paths: open(f"/data/{filename}"), path.join(base, user_path)
Safe	Path normalization + prefix check: os.path.realpath(path).startswith(BASE_DIR); allowlist of valid filenames
Look for	open(), read_file(), os.path.join with user input; ../ traversal without normalization

Insecure Direct Object Reference (IDOR)

Aspect	Detail
Vulnerable	API endpoint uses user-supplied ID without ownership check: GET /api/order/{order_id} — returns any user's order
Safe	Ownership verification: verify order.user_id == current_user.id before returning data
Look for	CRUD endpoints that accept IDs without authorization; horizontal/vertical privilege checks missing

Weak Cryptography

Aspect	Detail
Vulnerable	MD5/SHA1 for passwords; ECB mode; hardcoded keys; random module (not secrets); short key lengths
Safe	bcrypt/argon2 for passwords; AES-GCM; secrets module; RSA 2048+; proper IV generation
Look for	md5, sha1, DES, ECB, PKCS1_v1_5, random for crypto, hardcoded key=, Crypto.Cipher without AEAD

Secrets Exposure

Aspect	Detail
Vulnerable	Hardcoded API keys, passwords, tokens in source code; secrets in logs; secrets in client-side code
Safe	Environment variables; secret manager (AWS Secrets Manager, HashiCorp Vault); .env excluded from VCS
Look for	API_KEY=, password=, secret=, token= in code; AWS keys, GitHub tokens, Stripe keys, JWTs in source
False-positive guard	Secrets stored on disk but otherwise secured ARE excluded. Logging high-value secrets IS a vuln. Logging URLs is safe.

Template Injection (SSTI)

Aspect	Detail
Vulnerable	User input in template rendering: Template(user_input).render(), render_template_string(user_input)
Safe	Static templates; input passed as context variable, not template string
Look for	render_template_string, Template()() with user string; eval in template context; ${user_input} in JS template literals on server

NoSQL Injection

Aspect	Detail
Vulnerable	User input in MongoDB queries: db.users.find({username: user_input}) where input is {"$gt": ""}
Safe	Schema validation; type checking on query params; ORM sanitization
Look for	MongoDB $where, $gt, $regex from user input; raw mongo queries without type coercion