security-review

Security Review

The third review axis in auto-pi's review phase — alongside Standards (does the code follow the repo's conventions?) and Spec (does the code match what was asked for?). This axis answers: does the diff introduce security vulnerabilities?

Runs as a parallel sub-agent with fresh context — only the diff, not the builder's reasoning — so the security review is not anchored by the implementer's assumptions.

Smell catalog

Each smell reads what it is → how to spot it in the diff → how to fix. Match against the diff hunks:

1. Injection — user-controlled input reaches a dangerous sink without sanitization.

   • SQL injection — string-concatenated queries, unparameterized query(), ORM raw queries with user input. → parameterize; use prepared statements / parameterized query builders.
   • Command injection — user input in exec(), execSync(), spawn() shell mode, backticks, subprocess.run(shell=True). → use argument arrays (execFile, spawn without shell: true); never interpolate.
   • Template injection — user input in template engines (eval, ejs, jinja2, nunjucks) that execute code. → use logic-less templates or auto-escaping; never eval user content.
   • XSS — unescaped user input rendered in HTML (innerHTML, dangerouslySetInnerHTML, |safe in Django, {!! !!} in Blade). → escape on output; use framework auto-escaping; CSP headers.

2. Authentication / authorization — missing or bypassable access control.

   • Missing auth checks — a route, endpoint, or function that should require authentication but doesn't. → add the auth middleware/guard.
   • Privilege escalation — a user can perform actions above their role (horizontal or vertical). → check role/permission at the action site, not just the route.
   • IDOR (Insecure Direct Object Reference) — user can access another user's resource by changing an ID in the URL/params. → verify ownership: where("user_id = ?", currentUser.id).
   • Missing ownership checks — the code loads a resource by ID without checking the requester owns it. → add ownership scoping.