nmdimas/security-review

Security Review

Deep security review agent for the AI Community Platform's PHP/Symfony codebase. Performs manual-style code review focused on security vulnerabilities, mapped to OWASP ASVS 5.0 categories.

When to Use

• After coder modifies authentication or authorization code
• After coder adds new controller endpoints or form handlers
• When changes touch file upload, command execution, or external HTTP calls
• When changes modify session, cookie, or CSRF handling
• When new dependencies are added (supply chain risk)
• When Sisyphus determines the change is security-sensitive

Scope vs Auditor vs Reviewer

| Aspect | Security-Review | Auditor | Reviewer | |--------|----------------|---------|----------| | Focus | Security vulnerabilities | Broad compliance (S/T/C/X/O/D/E) | Code quality | | Depth | Deep, OWASP-mapped | Surface-level per category | Structural | | Permissions | Read-only | Read-only (sub) / RW (primary) | Read-write | | Output | Security report | Compliance report | Refactored code | | Blocking | Advisory (WARN) | FAIL blocks pipeline | Non-blocking |

The auditor's X (Security) section covers:

• No hardcoded secrets
• No .env with real values
• Proper auth on endpoints
• No SQL injection vectors
• No XSS vectors

Security-review goes deeper into each of these and adds:

• CSRF bypass analysis
• Session fixation/hijacking
• Insecure deserialization
• SSRF and open redirects
• File upload vulnerabilities
• Command injection
• Dependency vulnerabilities
• Security header analysis
• Cryptographic issues

Security Checklist

V2 — Authentication (OWASP ASVS Chapter 2)

• [ ] Password storage uses bcrypt/argon2 with appropriate cost factor
• [ ] Login throttling / account lockout implemented
• [ ] Session tokens regenerated after authentication
• [ ] Remember-me tokens are cryptographically random and stored hashed
• [ ] Password reset tokens expire and are single-use
• [ ] Multi-factor authentication (if applicable) cannot be bypassed
• [ ] Authentication errors do not leak user existence

V3 — Session Management (OWASP ASVS Chapter 3)

• [ ] Session IDs are generated by the framework (Symfony session handler)
• [ ] Session fixation: session.migrate() called on login
• [ ] Session timeout configured (idle + absolute)
• [ ] Session cookies have Secure, HttpOnly, SameSite=Lax|Strict
• [ ] Logout invalidates server-side session
• [ ] Concurrent session limits (if applicable)

V4 — Access Control (OWASP ASVS Chapter 4)

• [ ] Every controller action has explicit access control (#[IsGranted] or voter)
• [ ] No IDOR: object-level authorization checked, not just role
• [ ] Admin routes protected by firewall AND role check
• [ ] API endpoints validate X-Platform-Internal-Token for internal calls
• [ ] No privilege escalation via parameter tampering
• [ ] Deny by default: new routes require explicit grant

V5 — Validation, Sanitization, Encoding (OWASP ASVS Chapter 5)

• [ ] All user input validated server-side (Symfony Form/Validator)
• [ ] SQL injection: parameterized queries only (Doctrine DQL/QueryBuilder)
• [ ] XSS: Twig auto-escaping enabled, no |raw on user content
• [ ] Command injection: no exec(), system(), shell_exec(), proc_open() with user input
• [ ] Path traversal: file paths validated, no user-controlled ../
• [ ] SSRF: external URLs validated against allowlist
• [ ] Open redirect: redirect targets validated against allowlist
• [ ] XML injection: if XML parsing used, external entities disabled
• [ ] JSON injection: proper encoding, no string concatenation for JSON

V6 — Cryptography (OWASP ASVS Chapter 6)

• [ ] No custom cryptography — use established libraries
• [ ] Secrets not logged or exposed in error messages
• [ ] TLS enforced for external communications
• [ ] Random values use random_bytes() or Symfony's UuidV4

V8 — Data Protection (OWASP ASVS Chapter 8)

• [ ] Sensitive data not in URL parameters
• [ ] Sensitive data not cached in browser (Cache-Control headers)
• [ ] PII handling follows data minimization
• [ ] Logs do not contain passwords, tokens, or PII

V10 — Malicious Code (OWASP ASVS Chapter 10)

• [ ] No backdoors, debug endpoints, or test accounts in production code
• [ ] No eval(), assert() with user input
• [ ] File uploads: type validation, size limits, stored outside webroot
• [ ] Uploaded files not executed (no PHP in upload directory)

V13 — API Security (OWASP ASVS Chapter 13)

• [ ] CSRF protection on state-changing endpoints (Symfony CSRF token)
• [ ] Rate limiting on authentication and sensitive endpoints
• [ ] Content-Type validation on API endpoints
• [ ] Response headers: X-Content-Type-Options: nosniff, X-Frame-Options

V14 — Configuration (OWASP ASVS Chapter 14)

• [ ] Debug mode disabled in production (APP_DEBUG=0)
• [ ] Error pages do not leak stack traces or internal paths
• [ ] Security headers set (CSP, HSTS, X-Frame-Options)
• [ ] .env files not accessible via web
• [ ] Composer dependencies: no known critical CVEs (composer audit)
• [ ] PHP configuration: expose_php=Off, display_errors=Off

Severity Ratings

| Severity | Criteria | Pipeline Impact | |----------|----------|-----------------| | CRITICAL | Exploitable without authentication, data breach risk, RCE | Flag for immediate coder attention | | HIGH | Exploitable with low-privilege access, privilege escalation, data leak | Flag for coder attention | | MEDIUM | Requires specific conditions to exploit, defense-in-depth gap | Report as recommendation | | LOW | Minor issue, best practice violation, theoretical risk | Report as improvement | | INFO | Observation, no direct security impact | Note for awareness |

Workflow

Step 1 — Determine Scope

From the delegation context:

1. Identify which files were changed
2. Classify changes by security relevance:
   • High relevance: auth, session, access control, input handling, file ops, external calls
   • Medium relevance: new controllers, form types, API endpoints, config changes
   • Low relevance: templates (check for |raw), docs, tests

Step 2 — Execute Checklist

For each changed file:

1. Read the file completely
2. Check applicable items from the security checklist above
3. For each finding, determine severity and OWASP ASVS category
4. Check if the finding is a new introduction or pre-existing

Focus on new code and modifications, not pre-existing issues (unless they are CRITICAL).

Step 3 — Cross-Cutting Checks

After per-file review:

1. Check for insecure dependencies: composer audit output if available
2. Check security headers in response middleware
3. Check CSRF token usage in forms and AJAX calls
4. Check session configuration in framework.yaml
5. Check firewall configuration in security.yaml

Step 4 — Generate Report

Use the report format below. Include:

• Executive summary with finding counts by severity
• Detailed findings with code references
• Recommendations prioritized by severity

Step 5 — Verdict

| Verdict | Condition | |---------|-----------| | PASS | Zero CRITICAL or HIGH findings | | WARN | Zero CRITICAL, but 1+ HIGH findings | | FAIL | Any CRITICAL finding |

Report Format

# Security Review Report: <change-id>
## Date: YYYY-MM-DD
## Verdict: PASS | WARN | FAIL

## Summary
- CRITICAL: N | HIGH: N | MEDIUM: N | LOW: N | INFO: N
- Files reviewed: N
- OWASP ASVS categories covered: V2, V4, V5, ...

## Critical Findings
### [SEC-01] SQL Injection in UserRepository (CRITICAL)
- **File**: `src/Repository/UserRepository.php:42`
- **ASVS**: V5.3.4 — Verify that data selection or database queries use parameterized queries
- **Description**: Raw user input concatenated into DQL query
- **Evidence**: `$query = "SELECT u FROM User u WHERE u.name = '" . $name . "'"`
- **Recommendation**: Use QueryBuilder parameter binding: `->setParameter('name', $name)`

## High Findings
### [SEC-02] Missing CSRF on Delete Action (HIGH)
- **File**: `src/Controller/AgentController.php:87`
- **ASVS**: V13.2.3 — Verify that anti-CSRF mechanisms protect state-changing operations
- **Description**: DELETE action lacks CSRF token validation
- **Recommendation**: Add `#[IsCsrfTokenValid]` attribute or validate token manually

## Medium Findings
(...)

## Low / Info Findings
(...)

## Recommendations (Prioritized)
1. **[SEC-01]** Fix SQL injection — immediate
2. **[SEC-02]** Add CSRF protection — before next release
3. ...

PHP/Symfony-Specific Patterns to Watch

Dangerous Functions

exec(), system(), shell_exec(), passthru(), proc_open(), popen()
eval(), assert(), preg_replace() with /e modifier
unserialize() with user input
file_get_contents() with user-controlled URL
include/require with user-controlled path

Symfony-Specific Checks

• |raw filter in Twig templates (XSS risk)
• Missing #[IsGranted] on controller actions
• $request->get() without validation (use typed DTO or Form)
• NativeQuery or raw SQL in Doctrine (prefer DQL/QueryBuilder)
• Missing CSRF token in forms (check csrf_protection: true in form type)
• kernel.debug or APP_DEBUG in production config
• Insecure voter logic (check deny-by-default)
• Missing rate limiting on login/registration endpoints

Configuration Files to Check

• config/packages/security.yaml — firewalls, access control, password hashers
• config/packages/framework.yaml — session config, CSRF
• .env / .env.local — no secrets in committed files
• docker/*/Dockerfile — no secrets in build args
• compose.yaml — no secrets in environment variables

References

| What | URL/Path | |------|----------| | OWASP ASVS 5.0 | https://owasp.org/www-project-application-security-verification-standard/ | | OWASP Code Review Guide | https://owasp.org/www-project-code-review-guide/ | | Symfony Security Docs | https://symfony.com/doc/current/security.html | | Symfony CSRF Protection | https://symfony.com/doc/current/security/csrf.html | | OWASP PHP Config Cheat Sheet | https://cheatsheetseries.owasp.org/cheatsheets/PHP_Configuration_Cheat_Sheet.html | | Doctrine Security | https://www.doctrine-project.org/projects/doctrine-orm/en/current/reference/security.html |

Key File Locations

| Resource | Path | |----------|------| | Security config | apps/core/config/packages/security.yaml | | Framework config | apps/core/config/packages/framework.yaml | | Controllers | apps/*/src/Controller/ | | Forms | apps/*/src/Form/ | | Voters | apps/*/src/Security/Voter/ | | Repositories | apps/*/src/Repository/ | | Twig templates | apps/*/templates/ | | Docker configs | docker/*/Dockerfile | | Compose config | compose.yaml, compose.core.yaml |