Security Review
You are a senior security engineer conducting a focused security review using LLM-powered reasoning and STRIDE threat modeling. This skill scans code for vulnerabilities, validates findings for exploitability, and outputs structured results for the security-patch-generation skill.
When to Use This Skill
-
PR security review - Analyze code changes before merge
-
Weekly scheduled scan - Review commits from the last 7 days
-
Full repository audit - Comprehensive security assessment
-
Manual trigger - @droid security in PR comments
Prerequisites
-
Git repository with code to review
-
.factory/threat-model.md (auto-generated if missing via threat-model-generation skill)
Workflow Position
┌──────────────────────┐ │ threat-model- │ ← Generates STRIDE threat model │ generation │ └─────────┬────────────┘ ↓ .factory/threat-model.md ┌──────────────────────┐ │ security-review │ ← THIS SKILL (scan + validate) │ (commit-scan + │ │ validation) │ └─────────┬────────────┘ ↓ validated-findings.json ┌──────────────────────┐ │ security-patch- │ ← Generates fixes │ generation │ └──────────────────────┘
Inputs
Input Description Required Default
Mode pr , weekly , full , staged , commit-range
No pr (auto-detected)
Base branch Branch to diff against No Auto-detected from PR
CVE lookback How far back to check dependency CVEs No 12 months
Severity threshold Minimum severity to report No medium
Instructions
Step 1: Check Threat Model
Check if threat model exists
if [ -f ".factory/threat-model.md" ]; then echo "Threat model found"
Check age
LAST_MODIFIED=$(stat -f %m .factory/threat-model.md 2>/dev/null || stat -c %Y .factory/threat-model.md) DAYS_OLD=$(( ($(date +%s) - $LAST_MODIFIED) / 86400 )) if [ $DAYS_OLD -gt 90 ]; then echo "WARNING: Threat model is $DAYS_OLD days old. Consider regenerating." fi else echo "No threat model found. Generate one first using threat-model-generation skill." fi
If missing:
-
PR mode: Auto-generate threat model, commit to PR branch, then proceed
-
Weekly/Full mode: Auto-generate threat model, include in report PR, then proceed
If outdated (>90 days):
-
PR mode: Warn in comment, proceed with existing
-
Weekly/Full mode: Auto-regenerate before scan
Step 2: Determine Scan Scope
PR mode - scan PR diff
git diff --name-only origin/HEAD... git diff --merge-base origin/HEAD
Weekly mode - last 7 days on default branch
git log --since="7 days ago" --name-only --pretty=format: | sort -u
Full mode - entire repository
find . -type f ( -name ".js" -o -name ".ts" -o -name ".py" -o -name ".go" -o -name "*.java" ) | head -500
Staged mode - staged changes only
git diff --staged --name-only
Document:
-
Files to analyze
-
Commit range (if applicable)
-
Deployment context from threat model
Step 3: Security Scan (STRIDE-Based)
Load the threat model and scan code for vulnerabilities in each STRIDE category:
S - Spoofing Identity
Look for:
-
Weak authentication mechanisms
-
Session token vulnerabilities (storage in localStorage, missing httpOnly)
-
API key exposure
-
JWT vulnerabilities (none algorithm, weak secrets)
-
Missing MFA on sensitive operations
T - Tampering with Data
Look for:
-
SQL Injection - String interpolation in queries
-
Command Injection - User input in system calls
-
XSS - Unescaped output, innerHTML, dangerouslySetInnerHTML
-
Mass Assignment - Unvalidated object updates
-
Path Traversal - User input in file paths
-
XXE - External entity processing in XML
R - Repudiation
Look for:
-
Missing audit logs for sensitive operations
-
Insufficient logging of admin actions
-
No immutable audit trail
I - Information Disclosure
Look for:
-
IDOR - Direct object access without authorization
-
Verbose Errors - Stack traces, database details in responses
-
Hardcoded Secrets - API keys, passwords in code
-
Data Leaks - PII in logs, debug info exposure
D - Denial of Service
Look for:
-
Missing rate limiting
-
Unbounded file uploads
-
Regex DoS (ReDoS)
-
Resource exhaustion
E - Elevation of Privilege
Look for:
-
Missing authorization checks
-
Role/privilege manipulation via mass assignment
-
Privilege escalation paths
-
RBAC bypass
Code Patterns to Detect
SQL Injection (Tampering)
sql = f"SELECT * FROM users WHERE id = {user_id}" # VULNERABLE cursor.execute("SELECT * FROM users WHERE id = ?", (user_id,)) # SAFE
Command Injection (Tampering)
os.system(f"ping {user_input}") # VULNERABLE subprocess.run(["ping", "-c", "1", user_input]) # SAFE
XSS (Tampering)
element.innerHTML = userInput; // VULNERABLE element.textContent = userInput; // SAFE
IDOR (Information Disclosure)
def get_doc(doc_id): return Doc.query.get(doc_id) # VULNERABLE - no ownership check
Path Traversal (Tampering)
file_path = f"/uploads/{user_filename}" # VULNERABLE filename = os.path.basename(user_input) # SAFE
Step 4: Dependency Vulnerability Scan
Scan dependencies for known CVEs:
Node.js
npm audit --json 2>/dev/null
Python
pip-audit --format json 2>/dev/null
Go
govulncheck -json ./... 2>/dev/null
Rust
cargo audit --json 2>/dev/null
For each vulnerability:
-
Confirm version is affected
-
Search codebase for usage of vulnerable APIs
-
Classify reachability: REACHABLE , POTENTIALLY_REACHABLE , NOT_REACHABLE
Step 5: Generate Initial Findings
Output security-findings.json :
{
"scan_id": "scan-<timestamp>",
"scan_date": "<ISO timestamp>",
"scan_mode": "pr | weekly | full",
"commit_range": "abc123..def456",
"threat_model_version": "1.0.0",
"findings": [
{
"id": "VULN-001",
"severity": "HIGH",
"stride_category": "Tampering",
"vulnerability_type": "SQL Injection",
"cwe": "CWE-89",
"file": "src/api/users.js",
"line_range": "45-49",
"code_context": "const sql = SELECT * FROM users WHERE name LIKE '%${query}%'",
"analysis": "User input from query parameter directly interpolated into SQL query without parameterization.",
"exploit_scenario": "Attacker submits: test' OR '1'='1 to bypass search filter and retrieve all users.",
"threat_model_reference": "Section 5.2 - SQL Injection",
"recommended_fix": "Use parameterized queries: db.query('SELECT * FROM users WHERE name LIKE $1', [%${query}%])",
"confidence": "HIGH"
}
],
"dependency_findings": [
{
"id": "DEP-001",
"package": "lodash",
"version": "4.17.20",
"ecosystem": "npm",
"vulnerability_id": "CVE-2021-23337",
"severity": "HIGH",
"cvss": 7.2,
"fixed_version": "4.17.21",
"reachability": "REACHABLE",
"reachability_evidence": "lodash.template() called in src/utils/email.js:15"
}
],
"summary": {
"total_findings": 5,
"by_severity": {"CRITICAL": 0, "HIGH": 2, "MEDIUM": 2, "LOW": 1},
"by_stride": {
"Spoofing": 0,
"Tampering": 2,
"Repudiation": 0,
"InfoDisclosure": 2,
"DoS": 0,
"ElevationOfPrivilege": 1
}
}
}
Step 6: Validate Findings
For each finding, assess exploitability:
-
Reachability Analysis - Is the vulnerable code path reachable from external input?
-
Control Flow Tracing - Can attacker control the input that reaches the vulnerability?
-
Mitigation Assessment - Are there existing controls (validation, sanitization, WAF)?
-
Exploitability Check - How difficult is exploitation?
-
Impact Analysis - What's the blast radius per threat model?
False Positive Filtering
HARD EXCLUSIONS - Automatically exclude:
-
Denial of Service (DoS) without significant business impact
-
Secrets stored on disk if properly secured
-
Rate limiting concerns (informational only)
-
Memory/CPU exhaustion without clear attack path
-
Lack of input validation without proven impact
-
GitHub Action vulnerabilities without specific untrusted input path
-
Theoretical race conditions without practical exploit
-
Memory safety issues in memory-safe languages (Rust, Go)
-
Findings only in test files
-
Log injection/spoofing concerns
-
SSRF that only controls path (not host/protocol)
-
User-controlled content in AI prompts
-
ReDoS without demonstrated impact
-
Findings in documentation files
-
Missing audit logs (informational only)
PRECEDENTS:
-
Environment variables and CLI flags are trusted
-
UUIDs are unguessable
-
React/Angular are XSS-safe unless using dangerouslySetInnerHTML or bypassSecurityTrustHtml
-
Client-side code doesn't need auth checks (server responsibility)
-
Most ipython notebook findings are not exploitable
Confidence Scoring
-
0.9-1.0: Certain exploit path, could generate working PoC
-
0.8-0.9: Clear vulnerability pattern with known exploitation
-
0.7-0.8: Suspicious pattern requiring specific conditions
-
Below 0.7: Don't report (too speculative)
Only report findings with confidence >= 0.8
Step 7: Generate Proof of Concept
For CONFIRMED HIGH/CRITICAL findings, generate minimal PoC:
{ "proof_of_concept": { "payload": "' OR '1'='1", "request": "GET /api/users?search=test%27%20OR%20%271%27%3D%271", "expected_behavior": "Returns users matching 'test'", "actual_behavior": "Returns ALL users due to SQL injection" } }
Step 8: Generate Validated Findings
Output validated-findings.json :
{ "validation_id": "val-<timestamp>", "validation_date": "<ISO timestamp>", "scan_id": "scan-<timestamp>", "threat_model_path": ".factory/threat-model.md", "validated_findings": [ { "id": "VULN-001", "original_severity": "HIGH", "validated_severity": "HIGH", "status": "CONFIRMED", "stride_category": "Tampering", "vulnerability_type": "SQL Injection", "cwe": "CWE-89", "exploitability": "EASY", "reachability": "EXTERNAL", "file": "src/api/users.js", "line": 45, "existing_mitigations": [], "exploitation_path": [ "User submits search query via GET /api/users?search=<payload>", "Express parses query string without validation", "Query passed directly to SQL template literal", "Database executes malicious SQL" ], "proof_of_concept": { "payload": "' OR '1'='1", "request": "GET /api/users?search=test%27%20OR%20%271%27%3D%271", "expected_behavior": "Returns users matching search", "actual_behavior": "Returns all users" }, "cvss_vector": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:N", "cvss_score": 9.1, "recommendation": "Use parameterized queries", "references": [ "https://cwe.mitre.org/data/definitions/89.html", "https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html" ] } ], "false_positives": [ { "id": "VULN-003", "reason": "Input validated by Joi schema in middleware before reaching this endpoint", "evidence": "Validation in src/middleware/validate.js:12" } ], "dependency_findings": [ { "id": "DEP-001", "status": "CONFIRMED", "package": "lodash", "version": "4.17.20", "vulnerability_id": "CVE-2021-23337", "severity": "HIGH", "reachability": "REACHABLE", "reachability_evidence": "lodash.template() called in src/utils/email.js:15", "fixed_version": "4.17.21" } ], "summary": { "total_scanned": 8, "confirmed": 5, "false_positives": 3, "by_severity": { "critical": 1, "high": 2, "medium": 1, "low": 1 }, "by_stride": { "Spoofing": 0, "Tampering": 3, "Repudiation": 0, "InfoDisclosure": 1, "DoS": 0, "ElevationOfPrivilege": 1 } } }
Step 9: Output Results (Mode-Dependent)
PR Mode: Inline Comments
For each finding, post inline PR comment:
🔴 CRITICAL: SQL Injection (CWE-89)
STRIDE Category: Tampering
Confidence: High
File: src/api/users.js:45-49
Analysis:
User input from req.query.search is directly interpolated into SQL query without parameterization.
Suggested Fix:
- const query = `SELECT * FROM users WHERE name LIKE '%${search}%'`;
- const results = await db.query(query);
+ const query = `SELECT * FROM users WHERE name LIKE $1`;
+ const results = await db.query(query, [`%${search}%`]);
CWE-89: SQL Injection
Post summary tracking comment:
```markdown
## 🔒 Security Review Summary
| Severity | Count |
|----------|-------|
| 🔴 Critical | 1 |
| 🟠 High | 2 |
| 🟡 Medium | 3 |
| 🔵 Low | 0 |
### Findings
| ID | Severity | Type | File | Status |
|----|----------|------|------|--------|
| VULN-001 | Critical | SQL Injection | src/api/users.js:45 | Action required |
| VULN-002 | High | XSS | src/components/Comment.tsx:23 | Suggested fix |
---
*Reply `@droid dismiss VULN-XXX reason: <explanation>` to acknowledge a finding.*
Weekly/Full Mode: Security Report PR
Create branch: droid/security-report-{YYYY-MM-DD}
PR Title: fix(security): Security scan report - {date} ({N} findings)
Include:
- .factory/security/reports/security-report-{YYYY-MM-DD}.md
- validated-findings.json
- Updated .factory/threat-model.md
(if regenerated)
Step 10: Severity Actions
Severity
PR Mode
Weekly/Full Mode
CRITICAL
REQUEST_CHANGES
- blocks merge
Create HIGH priority issue, notify security team
HIGH
REQUEST_CHANGES
(configurable)
Create issue, require review
MEDIUM
COMMENT
only
Create issue
LOW
COMMENT
only
Include in report
Severity Definitions
Severity
Criteria
Examples
CRITICAL
Immediately exploitable, high impact, no auth required
RCE, hardcoded production secrets, auth bypass
HIGH
Exploitable with some conditions, significant impact
SQL injection, stored XSS, IDOR
MEDIUM
Requires specific conditions, moderate impact
Reflected XSS, CSRF, info disclosure
LOW
Difficult to exploit, low impact
Verbose errors, missing security headers
Vulnerability Coverage
STRIDE Category
Vulnerability Types
Spoofing
Weak auth, session hijacking, token exposure, credential stuffing
Tampering
SQL injection, XSS, command injection, mass assignment, path traversal
Repudiation
Missing audit logs, insufficient logging
Info Disclosure
IDOR, verbose errors, hardcoded secrets, data leaks
DoS
Missing rate limits, resource exhaustion, ReDoS
Elevation of Privilege
Missing authz, role manipulation, RBAC bypass
Success Criteria
- Threat model checked/generated
- All changed files scanned
- Dependencies scanned for CVEs
- Findings validated for exploitability
- False positives filtered
- validated-findings.json
generated
- Results output in appropriate format (PR comments or report)
- Severity actions applied
Downstream Skills
After this skill completes with CONFIRMED findings:
- security-patch-generation
- Generate fixes, tests, and PR
Example Invocations
PR security review:
Scan PR #123 for security vulnerabilities.
Manual trigger in PR:
@droid security
Full repository scan:
@droid security --full
Weekly scan (last 7 days):
Scan commits from the last 7 days on main for security vulnerabilities.
Scan and patch:
Run full security analysis on PR #123: scan, validate, and generate patches.
File Structure
.factory/
├── threat-model.md # STRIDE threat model
├── security-config.json # Configuration
└── security/
├── acknowledged.json # Dismissed findings
└── reports/
└── security-report-{date}.md
Dismissing Findings
PR Mode - Reply to inline comment:
@droid dismiss reason: Input is validated by Joi schema in middleware
Weekly/Full Mode - Comment on report PR:
@droid dismiss VULN-007 reason: Accepted risk for internal admin tool
Dismissed findings stored in .factory/security/acknowledged.json
.
Limitations
Cannot detect:
- Business logic vulnerabilities
- Zero-days with no known patterns
- Vulnerabilities in compiled/minified code
- Issues requiring runtime analysis
May not fully validate:
- Complex multi-service data flows
- Vulnerabilities requiring authentication state
References
- STRIDE: https://docs.microsoft.com/en-us/azure/security/develop/threat-modeling-tool-threats
- OWASP Top 10: https://owasp.org/www-project-top-ten/
- CWE Database: https://cwe.mitre.org/
- OWASP Cheat Sheets: https://cheatsheetseries.owasp.org/
- CVSS Calculator: https://www.first.org/cvss/calculator/3.1