ds-security-review

When invoked, audit the code in scope against one question: how would an attacker abuse this? Look for the weaknesses that lead to real compromise — injection, broken access control, leaked secrets, untrusted input trusted too far. This is the portable, cross-language pass; for deeper language specifics, /ds-go-review, /ds-ts-review, and /ds-rust-review carry their own Security sections. Every finding names the attack that exploits it. Report-only by default — no edits unless --fix is passed.

Arguments

• Treat positional args as scope (files, directories, globs). With no scope, review the code changed on the current branch.
• --full → review the entire codebase instead of just the branch's changes. Explicit positional scope still wins; --full only replaces the no-scope default.
• Freeform scope ("the auth handler", "the upload path") is interpreted reasonably.
• --fix → after reporting, apply only the findings whose fix is mechanical and unambiguous — a single obvious edit, no design judgment (e.g. removing a secret committed to source, tightening over-permissive file modes). A wrong fix to a security finding is worse than none, so anything that changes behavior or rests on an assumption you couldn't verify stays report-only. After applying, re-run any build/test/lint check already in the loop and revert any fix that breaks it — or that touched more than the intended mechanical edit. Close with a summary of what was applied and what was left.

Tool-aided passes (when available — additive)

Use these as mechanical aids, not replacements for reading the code. A match is a lead; you still trace it to confirm exploitability.

• ast-grep (command -v ast-grep): read the companion ast-grep.md and run a structural pass. It enumerates known-dangerous patterns — injection sinks, risky-API call sites, untrusted-type flows — that a skim of a large diff can miss. If ast-grep isn't installed, review normally.
• /ds-semgrep (command -v semgrep): run a local SAST scan for fast, rule-based security findings — source SAST to /ds-osv's dependency SCA. If semgrep isn't installed, skip the SAST step and rely on the manual review.

If neither tool is installed, the full read is still the baseline.

Depth pass (conditional)

• authn.md — read the companion when the scope touches login, registration, password reset or change, email/phone change, session issuance, MFA enrolment, or an OAuth/OIDC/SAML integration. It carries the identity-specific failure modes the generic checks below don't reach: password-policy thresholds, constant-time comparison, the three enumeration leaks (message, status, response time), lockout that becomes a denial of service, re-authentication before sensitive actions, and the pending-change email flow. Skip it when the scope has no identity code.

What to check

Trace untrusted data from where it enters to where it's used. Most vulnerabilities are an input that reaches a dangerous sink without validation in between.

1. Injection. Untrusted input reaching an interpreter — SQL/NoSQL, OS commands, file paths (traversal), URLs (SSRF), templates, eval-like calls, LDAP. Look for string-built queries or commands instead of parameterized/escaped APIs. An LLM that can call tools is an interpreter too: untrusted content reaching its context is the same weakness with a different sink — flag it here and hand the system-level review to /ds-agent-review.

2. Output handling. Untrusted data rendered without context-correct encoding (XSS), unsafe deserialization of attacker-controlled data, content-type confusion.

3. Access control. Missing or wrong authorization on an action; object-level checks absent (IDOR — can user A reach user B's record?); privilege escalation; trusting a client-supplied role or id; an auth check bypassable by ordering or a missing branch.

4. Secrets & crypto. Hardcoded credentials, keys, or tokens; secrets in logs, errors, or responses; rolled-own or weak crypto; predictable randomness used for security (tokens, IDs, salts); missing encryption for sensitive data in transit or at rest.

5. Sensitive-data exposure. PII or secrets in logs, stack traces, or verbose errors returned to the caller; over-broad API responses; debug endpoints or stack traces reachable in production paths.

6. Untrusted input trusted too far. Mass assignment / binding attacker-controlled fields; unvalidated redirects; unsafe file upload (type, size, path); unbounded input enabling resource exhaustion (DoS) — allocation, recursion, regex backtracking; unbounded work that costs money rather than CPU (metered API calls, LLM inference, egress) — denial of wallet.

7. Configuration & transport. Missing TLS or certificate validation, permissive CORS, missing security headers, default credentials, overly broad permissions or IAM.

8. Dependencies. Known-vulnerable or untrusted dependencies introduced by the change. (The language reviews run the deeper audit tooling — flag the obvious here.)

Output

A prioritized findings list, ordered by exploitability × impact:

1. Critical — directly exploitable for code execution, data breach, or auth bypass
2. High — exploitable under realistic conditions, or a clear data-exposure path
3. Hardening — defense-in-depth gap, not directly exploitable on its own

For each finding:

• Anchor to file:line.
• State the weakness in one line, describe the attack that exploits it (the input and the sink), then the fix — prefer the standard safe API over hand-rolled escaping.
• Note your confidence and any assumption about what's trusted.

Rules:

• Exploitable findings over theoretical ones. Name a path from attacker-controlled input to impact; a "weakness" on a fully-trusted internal path is hardening at most.
• A short, high-confidence list beats a long speculative one.
• With --fix, additionally summarize what was applied vs. left (mechanics in Arguments).