# Security Review

> Security-focused PR review for vulnerabilities and best practices. Invokes the Security Reviewer agent (Hakim).

- Skill: `me2resh/security-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add me2resh/security-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/me2resh/security-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: AI & ML
- Author: me2resh (https://skillmd.com/u/me2resh)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/me2resh/security-review

---


## Writing rule

When this skill writes a durable artifact, read .claude/rules/writing-standard.md. Use the controlled technical writing profile.

# /security-review — Security Review

Review a pull request specifically for security vulnerabilities and best practices.

## LSP-aware (optional, recommended)

This skill performs semantic code navigation — finding definitions, walking references, tracing handlers across modules. With LSP enabled (`ENABLE_LSP_TOOL=1` + per-language plugin per `docs/getting-started.md`), queries are ~3-15× cheaper in token cost than grep + Read. Without LSP, the skill falls back to grep + Read transparently — no new failure mode, just optional speed.

Per-language LSP plugins live in Claude Code's marketplace. Install once; the skill detects the active language and dispatches automatically.

## Activated agent + role

When `/security-review` runs:

1. **Primary reviewer**: the **Security Reviewer agent (Hakim)** at [`.claude/agents/security-reviewer.md`](../../agents/security-reviewer.md) — runs the automated security checklist.
2. **Human approval gate**: the **[Security Auditor](../../../roles/security/security-auditor.md)** role — activates on any PR that touches auth / crypto / secrets / user data / PII, or when `/security-review` is explicitly invoked.
3. **Escalation for strategic calls**: the **[Head of Security](../../../roles/security/head-of-security.md)** — threat modelling, compliance decisions, or novel attack surfaces.
4. **For active testing**: the **[Penetration Tester](../../../roles/security/penetration-tester.md)** — exploit discovery, API security review, pre-release security sign-off.

See [`.claude/rules/role-triggers.md`](../../rules/role-triggers.md) for the full activation protocol.

## Usage

```
/security-review 42
/security-review 42 your-org/your-repo
```

## When to Use

Invoke for PRs that touch:

- Authentication / authorisation
- User input handling
- API endpoints
- Data storage
- Third-party integrations
- Cryptography or secrets

## Process

### 0. Write the active-reviewer marker (REQUIRED — me2resh/apexyard#843)

Before spawning the Security Reviewer agent, write the active-reviewer session marker. It records that this review pass is the sanctioned one and suppresses `warn-review-marker-write.sh`'s advisory warning on the `*-security.approved` write (same convention as `/code-review`'s rex marker; that hook warns and never blocks since #1026 — AgDR-0111). At skill entry:

```bash
ops_root=$(git rev-parse --show-toplevel)
r="$ops_root"
while [ -n "$r" ] && [ "$r" != "/" ]; do
  [ -f "$r/.apexyard-fork" ] && { ops_root="$r"; break; }
  [ -f "$r/onboarding.yaml" ] && [ -f "$r/apexyard.projects.yaml" ] && { ops_root="$r"; break; }
  r=$(dirname "$r")
done
mkdir -p "$ops_root/.claude/session"
printf '%s\n' "<owner/repo>#<pr>:security" > "$ops_root/.claude/session/active-reviewer"
```

On skill exit (after the review is posted), clear the marker:

```bash
rm -f "$ops_root/.claude/session/active-reviewer"
```

Nothing mechanically stops a build-class sub-agent writing the same file; what makes this marker legitimate is that a real, independent review happened. See `.claude/hooks/warn-review-marker-write.sh` and `.claude/rules/pr-workflow.md` § "Build agents cannot self-review".

### 0a. Never hand the reviewer a marker path (me2resh/apexyard#1144)

**The spawn prompt for Hakim MUST NOT contain a literal marker path.** Say
*"write your approval marker on an APPROVED verdict"*; say nothing about where.

Hakim already resolves the correct path through `review_marker_path` — the
repo-qualified `<owner>__<repo>__<pr>-security.approved` form from AgDR-0060,
which is the exact path the gates read. A path in the prompt overrides that
correct resolution: the agent obeys the instruction it was handed, and the
marker lands at the bare-number `<pr>-security.approved` instead. **No gate reads
that path** — there is no bare-number fallback on any on-disk marker lookup.

The failure is silent in the dangerous direction. `ls .claude/session/reviews/`
shows a file that reads, to a human, like a valid approval; only the merge
attempt reveals otherwise. And at that moment the obvious repair — moving the
file into place — is marker forging, the behaviour
[`pr-workflow.md`](../../rules/pr-workflow.md) § "Build agents cannot
self-review" exists to prevent. The right recovery is always: delete the
gate-invisible file and re-run a real review.

`warn-unqualified-review-marker.sh` warns (advisory, never blocks) when a
bare-number marker appears, and the merge gates name the near-miss in their
refusal message — but the cheap fix is upstream of both: don't pass a path.

## Security Checklist

> Baseline: OWASP Top 10 (2025) — supply-chain failures are now #3; use OWASP ASVS 5.0 as the verification baseline for these checks.

### Secrets & Credentials

- No hardcoded secrets, API keys, or passwords
- Environment variables for sensitive data
- No secrets in logs or error messages

### Injection Prevention

- Parameterised queries (no SQL injection)
- No command injection
- No template injection

### XSS Prevention

- User input sanitised before rendering
- No unsafe `dangerouslySetInnerHTML`
- No `eval()` with user input

### Authentication & Authorisation

- Auth checks on protected routes
- Authorisation verified before data access
- Secure session management

### Data Protection

- Sensitive data encrypted
- No PII in URLs or query strings
- Proper validation and sanitisation

### API Security

- Rate limiting considered
- Input validation on endpoints
- No stack traces exposed
- CORS configured correctly

## Severity Levels

| Level | Action |
|-------|--------|
| CRITICAL | Block PR immediately |
| HIGH | Block PR, require fix |
| MEDIUM | Warn, recommend fix |
| LOW | Informational |

## Output

Posts a GitHub review with:

- Commit SHA
- Security checklist results
- Issues with severity
- Verdict

Invokes: Security Reviewer Agent (Hakim)

## Persist the run + render trend

After the Security Reviewer agent posts the review — through the tracker-agnostic `tracker_review_submit` (gh PR / glab MR / custom host — #763), not a hardcoded `gh pr review` — persist a structured artefact via the shared audit-history lib so the security-review trend across PRs becomes legible. See `docs/agdr/AgDR-0019-audit-artefact-persistence.md` for the schema rationale.

### 1. Resolve project name + score + verdict

`<project-name>` is the project's registered name in `apexyard.projects.yaml`, derived from the PR's repo. If the project isn't registered, use the basename of the repo and tell the operator to `/handover` it for cross-machine trend continuity.

Compute a single headline score from the severity distribution of the findings in the review:

```
score = max(0, 100 - 25*critical - 10*high - 3*medium - 1*low)
```

Compute the verdict by the worst-severity rule:

| Worst severity present | Verdict |
|---|---|
| critical or high       | `fail` |
| medium only            | `conditional` |
| low only / none        | `pass` |

### 2. Build payload + body, persist via the lib

```bash
source "$(git rev-parse --show-toplevel)/.claude/hooks/_lib-audit-history.sh"

# Lowercase severity in the payload — the lib's stats derivation expects
# critical / high / medium / low / info. The visible review on the PR
# can use whatever capitalisation reads best.
payload=$(mktemp); cat > "$payload" <<'EOF'
{
  "schema_version": 1,
  "findings": [
    {"id": "F1", "severity": "critical", "status": "open", "summary": "Unsanitised user input in SQL"},
    {"id": "F2", "severity": "high",     "status": "open", "summary": "JWT signature not verified"}
  ]
}
EOF

# Body: a markdown summary of the security review for this PR, formatted
# per templates/audits/security-review.md. Include the diff scope, findings
# table, dependency vulnerabilities, secrets-scan results, and recommendations.
body=$(mktemp); cat > "$body" <<'EOF'
## Scope

PR <number>; reviewed `<branch>..main` (X files, Y +/- lines).

## Findings

| # | Severity | OWASP class | Finding | File:Line | Status |
|---|---|---|---|---|---|
| F1 | critical | A03 Injection | Unsanitised user input concatenated into SQL | `src/users.ts:42` | open |
| F2 | high | A07 Auth failure | JWT signature not verified | `src/auth.ts:18` | open |

## Dependency vulnerabilities

(... output of `npm audit` or `pip audit` for critical+high ...)

## Secrets scan

(... checks per templates/audits/security-review.md ...)

## Recommendations

1. F1 — fix injection vector before merge
2. F2 — verify JWT signatures
EOF

ts=$(date -u +"%Y-%m-%dT%H:%M:%SZ")
audit_run_persist "<project-name>" "security-review" "$ts" "fail" 50 "$body" < "$payload"
rm -f "$payload" "$body"
```

### 3. Render the trend section

```bash
audit_render_trend "<project-name>" "security-review" 5
```

- < 2 prior runs → silent (no trend section). Don't append anything.
- ≥ 2 prior runs → prints a markdown trend block (heading + table + ASCII chart of `score` over time) to stdout. Append it to this run's MD artefact so the PR-by-PR security trend is visible.

### 4. Opt-in commit (history-tracked marker)

By default the dimension's runs/ JSON files are gitignored. The lib applies a `.gitignore` based on the presence of the marker:

```bash
# Opt in to commit security-review history for this project
touch projects/<name>/audits/security-review/.audit-history-tracked
```

The MD artefacts at `<dim_dir>/<ts>.md` are committed regardless — they are the durable human-readable artefact of every PR's security review.

---

*Part of [ApexYard](https://github.com/me2resh/apexyard) — multi-project SDLC framework for Claude Code · MIT.*