--- name: eresus-pr-security-review description: > Security-focused pull request and diff review skill for finding newly introduced vulnerabilities, risky regressions, and missing security tests in changed code. Trigger when the user asks to: "review this PR for security", "check this diff for vulns", "do a security code review", "audit changed files", or wants findings on a patch instead of a full-repo scan. Best used alongside eresus-sast-scanner. metadata: version: "1.0" domain: application-security mode: review --- # Security PR Review ## Purpose Review changed code with a security-review mindset: prioritize introduced bugs, exploitable regressions, and missing coverage in the modified attack surface. Focus on what the diff changes, not on writing a general audit of the whole repository. ## Scope Rules - Prioritize vulnerabilities introduced or exposed by the diff. - Inspect surrounding code when needed to confirm reachability, auth context, or sanitizer behavior. - Mention pre-existing issues only when the change makes them reachable, worse, or security-relevant now. - Findings come first; summary is secondary. --- ## Workflow ### Step 1: Read the Diff as an Attack Surface Change Identify: - newly added endpoints, handlers, controllers, jobs, or CLI paths - changed authorization checks or middleware wiring - new deserialization, template rendering, file, network, or command execution paths - config changes that weaken defaults or widen trust boundaries ### Step 2: Map New Sources, Sinks, and Guards For changed code, locate: - attacker-controlled inputs - dangerous sinks - sanitization, validation, auth, and feature-flag guards Do not stop at the edited hunk if the actual sink or protection lives nearby. ### Step 3: Trace Security-Sensitive Data Flow Apply lightweight taint reasoning through: - helper calls - serializers and DTOs - middleware layers - ORM/query helpers - template/view rendering - background job enqueue/dequeue boundaries ### Step 4: Review Diff-Specific Risk Patterns Pay extra attention to: - string-built queries, filters, or shell commands - unsafe deserialization, polymorphic typing, object revival, or parser feature flags - auth checks moved later in the flow - newly trusted headers, cookies, or client-supplied role fields - open redirects added through `next`, `returnTo`, or `redirect_url` - debug logging of secrets, tokens, or PII - upload handling, archive extraction, or path joins - new outbound HTTP fetches or webhook callbacks - security settings changed from strict to permissive ### Step 5: Judge Before Reporting For every candidate finding, verify: - the input is actually attacker-controlled - the changed path is reachable - protections are not already effective - the issue is materially exploitable Drop speculative findings that cannot survive this check. ### Step 6: Report Findings and Testing Gaps Report confirmed issues with: - severity - exact changed file and line - exploit path in one short paragraph - concrete remediation direction - missing test coverage if the diff should have added one If no confirmed issue exists, say so explicitly and note any residual uncertainty or testing gaps. --- ## Review Guardrails - Do not file a security finding purely because code "looks risky"; explain the actual exploit path. - Do not ignore new trust boundaries just because the sink is in an unchanged helper. - Do not report infra-only assumptions as app bugs unless the diff depends on them. - Do not downgrade auth bugs simply because the endpoint is "internal" without evidence. - Do not confuse code-quality concerns with security findings unless they create exploitability. --- ## Output Format Use this structure: ```markdown [SEVERITY] File: : Why it matters: Fix: Tests: ``` When there are no findings: ```markdown No confirmed security findings in the reviewed diff. Residual risk: Testing gap: ```