Make security-review hooks and skill installable from the repo
The global pre-push hook, the /sh-security-review prompt, and finding.schema.json previously lived only in ~/.config/git and ~/.claude (untracked) — unreproducible. Source them here: add hooks/pre-push, rewrite install-hooks.sh with a --global mode (lays down both hooks, sets core.hooksPath, links skill+schema into ~/.claude) and a per-repo mode. Align pre-commit with pre-push (honor skip marker + suppressions). Add semgrep p/javascript so the scanners cover the org's Node/.NET repos.
This commit is contained in:
parent
18412c7482
commit
a3ab3f5f40
6 changed files with 292 additions and 19 deletions
69
security-review/finding.schema.json
Normal file
69
security-review/finding.schema.json
Normal file
|
|
@ -0,0 +1,69 @@
|
|||
{
|
||||
"$schema": "http://json-schema.org/draft-07/schema#",
|
||||
"title": "Sea Haven security-review finding",
|
||||
"description": "Structured finding contract for /sh-security-review. Same shape for interactive (Path A) and automated (Path B) runs, and the input review.sh reads to make the block decision.",
|
||||
"type": "object",
|
||||
"required": ["findings", "summary"],
|
||||
"properties": {
|
||||
"findings": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
"type": "object",
|
||||
"required": ["id", "title", "severity", "cwe", "file", "category", "data_flow", "proof", "status"],
|
||||
"properties": {
|
||||
"id": { "type": "string", "description": "stable slug, e.g. sqli-payment-handler-get-payment" },
|
||||
"title": { "type": "string" },
|
||||
"severity": {
|
||||
"type": "string",
|
||||
"enum": ["critical", "high", "medium", "low", "info", "unverified"],
|
||||
"description": "unverified = a claim with no accepted proof; auto-downgraded from its claimed severity"
|
||||
},
|
||||
"claimed_severity": {
|
||||
"type": "string",
|
||||
"enum": ["critical", "high", "medium", "low", "info"],
|
||||
"description": "the detector's original severity before the verifier ruled"
|
||||
},
|
||||
"cwe": { "type": "string", "pattern": "^CWE-[0-9]+$" },
|
||||
"file": { "type": "string" },
|
||||
"line": { "type": ["integer", "null"] },
|
||||
"category": {
|
||||
"type": "string",
|
||||
"enum": ["injection", "authz", "secrets-crypto", "iac-iam", "web-client", "logic", "other"]
|
||||
},
|
||||
"data_flow": {
|
||||
"type": "string",
|
||||
"description": "numbered plain-English trace from untrusted source to dangerous sink"
|
||||
},
|
||||
"proof": {
|
||||
"type": "object",
|
||||
"required": ["input", "outcome"],
|
||||
"properties": {
|
||||
"input": { "type": "string", "description": "concrete malicious input / trigger" },
|
||||
"outcome": { "type": "string", "description": "the specific bad result it produces" },
|
||||
"test": { "type": ["string", "null"], "description": "optional failing-test sketch" }
|
||||
}
|
||||
},
|
||||
"status": {
|
||||
"type": "string",
|
||||
"enum": ["confirmed", "unverified", "suppressed"],
|
||||
"description": "confirmed = verifier accepted proof; unverified = no accepted proof; suppressed = dismissed with justification"
|
||||
},
|
||||
"suppression_justification": {
|
||||
"type": ["string", "null"],
|
||||
"description": "REQUIRED when status=suppressed; logged and surfaced in the report"
|
||||
},
|
||||
"recommendation": { "type": "string" }
|
||||
}
|
||||
}
|
||||
},
|
||||
"summary": {
|
||||
"type": "object",
|
||||
"required": ["confirmed_critical", "confirmed_high", "block"],
|
||||
"properties": {
|
||||
"confirmed_critical": { "type": "integer" },
|
||||
"confirmed_high": { "type": "integer" },
|
||||
"block": { "type": "boolean", "description": "true if any confirmed critical/high is unsuppressed (the gate condition)" }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,9 +1,11 @@
|
|||
#!/usr/bin/env bash
|
||||
# Sea Haven security-review pre-commit hook: FAST deterministic scanners only (sub-30s).
|
||||
# The full agentic review is the on-demand /sh-security-review slash command — run that before pushing.
|
||||
# CI re-runs review.sh as the unbypassable backstop, so --no-verify here only skips local fast feedback.
|
||||
set -euo pipefail
|
||||
REPO_ROOT="$(git rev-parse --show-toplevel)"
|
||||
# Honors the same skip/suppress controls as pre-push so a suppressed FP doesn't block the commit.
|
||||
# --no-verify skips this local fast feedback; the pre-push hook + nightly VM sweep are the backstop.
|
||||
set -uo pipefail
|
||||
REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null)" || exit 0
|
||||
[ -f "$REPO_ROOT/.security-review-skip" ] && exit 0
|
||||
REVIEW_SH="${SH_REVIEW_SH:-$HOME/Documents/repositories/orchestrator/security-review/review.sh}"
|
||||
if [ ! -f "$REVIEW_SH" ]; then
|
||||
echo "security-review: review.sh not found at $REVIEW_SH (set SH_REVIEW_SH to override) — skipping" >&2
|
||||
|
|
@ -11,4 +13,7 @@ if [ ! -f "$REVIEW_SH" ]; then
|
|||
fi
|
||||
# Nothing staged -> nothing to do.
|
||||
git diff --cached --name-only --diff-filter=ACM | grep -q . || exit 0
|
||||
bash "$REVIEW_SH" --scanners-only "$REPO_ROOT"
|
||||
SUP=()
|
||||
[ -f "$REPO_ROOT/.security-review/suppressions.json" ] && SUP=(--suppressions "$REPO_ROOT/.security-review/suppressions.json")
|
||||
# ${SUP[@]+"${SUP[@]}"} = bash-3.2-safe expansion of a possibly-empty array under set -u.
|
||||
exec bash "$REVIEW_SH" --scanners-only ${SUP[@]+"${SUP[@]}"} "$REPO_ROOT"
|
||||
|
|
|
|||
26
security-review/hooks/pre-push
Executable file
26
security-review/hooks/pre-push
Executable file
|
|
@ -0,0 +1,26 @@
|
|||
#!/usr/bin/env bash
|
||||
# Sea Haven global pre-push security gate — fast deterministic scanners (review.sh --scanners-only).
|
||||
# Installed via install-hooks.sh --global: lays this down at ~/.config/git/hooks/pre-push and sets
|
||||
# git config --global core.hooksPath ~/.config/git/hooks
|
||||
# Skip a repo: add a .security-review-skip file at its root. Bypass once: git push --no-verify.
|
||||
# Deep agentic pass = on-demand /sh-security-review; nightly VM sweep = the backstop.
|
||||
set -uo pipefail
|
||||
REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null)" || exit 0
|
||||
[ -f "$REPO_ROOT/.security-review-skip" ] && exit 0
|
||||
REVIEW_SH="${SH_REVIEW_SH:-$HOME/Documents/repositories/orchestrator/security-review/review.sh}"
|
||||
if [ -f "$REVIEW_SH" ]; then
|
||||
SUP=()
|
||||
[ -f "$REPO_ROOT/.security-review/suppressions.json" ] && SUP=(--suppressions "$REPO_ROOT/.security-review/suppressions.json")
|
||||
echo "security-review: scanning $REPO_ROOT (scanners-only) before push..." >&2
|
||||
# ${SUP[@]+"${SUP[@]}"} = bash-3.2-safe expansion of a possibly-empty array under set -u.
|
||||
if ! bash "$REVIEW_SH" --scanners-only ${SUP[@]+"${SUP[@]}"} "$REPO_ROOT"; then
|
||||
echo "security-review: BLOCKED (confirmed crit/high). Fix it, suppress with justification, or 'git push --no-verify' to override." >&2
|
||||
exit 1
|
||||
fi
|
||||
else
|
||||
echo "security-review: review.sh not found at $REVIEW_SH (set SH_REVIEW_SH) — skipping gate" >&2
|
||||
fi
|
||||
# Don't silently disable a repo-local pre-push hook: chain to it if present.
|
||||
LOCAL_HOOK="$REPO_ROOT/.git/hooks/pre-push"
|
||||
[ -x "$LOCAL_HOOK" ] && exec "$LOCAL_HOOK" "$@"
|
||||
exit 0
|
||||
|
|
@ -1,12 +1,83 @@
|
|||
#!/usr/bin/env bash
|
||||
# Install the Sea Haven security-review pre-commit hook into a target repo.
|
||||
# Usage: install-hooks.sh /path/to/repo
|
||||
# install-hooks.sh — install the Sea Haven security-review git hooks + skill assets.
|
||||
#
|
||||
# Two modes:
|
||||
# install-hooks.sh --global Lay the hooks down once for EVERY repo on this machine:
|
||||
# writes ~/.config/git/hooks/{pre-commit,pre-push}, sets
|
||||
# git config --global core.hooksPath, and links the skill
|
||||
# prompt + finding.schema.json into ~/.claude (Path A).
|
||||
# install-hooks.sh /path/to/repo Per-repo install: copy the hooks into <repo>/.git/hooks
|
||||
# (use when a repo sets its own local core.hooksPath, e.g.
|
||||
# husky, which would otherwise shadow the global hook).
|
||||
# install-hooks.sh --help
|
||||
#
|
||||
# The hooks run review.sh --scanners-only (fast, deterministic). The full agentic review is the
|
||||
# on-demand /sh-security-review skill; the nightly VM sweep is the backstop. Idempotent + re-runnable.
|
||||
set -euo pipefail
|
||||
REPO="${1:?usage: install-hooks.sh /path/to/repo}"
|
||||
SRC="$(cd "$(dirname "$0")" && pwd)/hooks/pre-commit"
|
||||
[ -d "$REPO/.git" ] || { echo "not a git repo: $REPO" >&2; exit 1; }
|
||||
HOOK="$REPO/.git/hooks/pre-commit"
|
||||
if [ -f "$HOOK" ]; then echo "warning: existing pre-commit hook at $HOOK will be overwritten" >&2; fi
|
||||
cp "$SRC" "$HOOK"; chmod +x "$HOOK"
|
||||
echo "installed security-review pre-commit hook -> $HOOK"
|
||||
echo "note: hook runs deterministic scanners only; full review is /sh-security-review (on demand)."
|
||||
|
||||
SRC="$(cd "$(dirname "$0")" && pwd)"
|
||||
GLOBAL_HOOKS="$HOME/.config/git/hooks"
|
||||
CLAUDE_DIR="$HOME/.claude"
|
||||
|
||||
usage() { grep '^#' "$0" | sed 's/^# \{0,1\}//'; }
|
||||
|
||||
install_one() { # install_one <src-hook> <dest-hook>
|
||||
local src="$1" dest="$2"
|
||||
cp "$src" "$dest"
|
||||
chmod +x "$dest"
|
||||
}
|
||||
|
||||
link_asset() { # link_asset <src-file> <dest-path> (symlink so the repo stays source of truth)
|
||||
local src="$1" dest="$2"
|
||||
mkdir -p "$(dirname "$dest")"
|
||||
ln -sfn "$src" "$dest"
|
||||
echo " linked $dest -> $src"
|
||||
}
|
||||
|
||||
case "${1:-}" in
|
||||
-h|--help|"") usage; exit 0;;
|
||||
|
||||
--global)
|
||||
echo "== Installing Sea Haven security-review hooks globally =="
|
||||
mkdir -p "$GLOBAL_HOOKS"
|
||||
|
||||
# Warn (don't clobber silently) if a different global hooksPath is already set.
|
||||
CURRENT="$(git config --global --get core.hooksPath || true)"
|
||||
if [ -n "$CURRENT" ] && [ "$CURRENT" != "$GLOBAL_HOOKS" ]; then
|
||||
echo " WARNING: git config --global core.hooksPath is already '$CURRENT'." >&2
|
||||
echo " Overwriting it with '$GLOBAL_HOOKS'. Re-point manually if that was intentional." >&2
|
||||
fi
|
||||
|
||||
install_one "$SRC/hooks/pre-commit" "$GLOBAL_HOOKS/pre-commit"
|
||||
install_one "$SRC/hooks/pre-push" "$GLOBAL_HOOKS/pre-push"
|
||||
git config --global core.hooksPath "$GLOBAL_HOOKS"
|
||||
echo " installed pre-commit + pre-push -> $GLOBAL_HOOKS"
|
||||
echo " set git config --global core.hooksPath = $GLOBAL_HOOKS"
|
||||
|
||||
# Path A assets: link the on-demand skill prompt + finding schema into ~/.claude.
|
||||
link_asset "$SRC/skill/sh-security-review.md" "$CLAUDE_DIR/commands/sh-security-review.md"
|
||||
link_asset "$SRC/finding.schema.json" "$CLAUDE_DIR/security-review/finding.schema.json"
|
||||
|
||||
echo
|
||||
echo "Done. Every repo on this machine is now gated by review.sh --scanners-only before push."
|
||||
echo "Caveats: a repo that sets its OWN local core.hooksPath (e.g. husky) overrides this global hook"
|
||||
echo " — run 'install-hooks.sh <that-repo>' to gate it per-repo. Skip a repo with a"
|
||||
echo " .security-review-skip file at its root; bypass once with 'git push --no-verify'."
|
||||
;;
|
||||
|
||||
--*)
|
||||
echo "unknown option: $1" >&2; usage; exit 2;;
|
||||
|
||||
*)
|
||||
REPO="$1"
|
||||
[ -d "$REPO/.git" ] || { echo "not a git repo: $REPO" >&2; exit 1; }
|
||||
echo "== Installing security-review hooks into $REPO/.git/hooks =="
|
||||
for h in pre-commit pre-push; do
|
||||
DEST="$REPO/.git/hooks/$h"
|
||||
[ -f "$DEST" ] && echo " warning: existing $h hook at $DEST will be overwritten" >&2
|
||||
install_one "$SRC/hooks/$h" "$DEST"
|
||||
echo " installed $h -> $DEST"
|
||||
done
|
||||
echo "note: hooks run deterministic scanners only; full review is /sh-security-review (on demand)."
|
||||
;;
|
||||
esac
|
||||
|
|
|
|||
|
|
@ -67,7 +67,7 @@ SCOPE_PATHS="$(scope_paths)"
|
|||
# --- semgrep (SAST: injection/authz/xss/secrets) ---
|
||||
if command -v semgrep >/dev/null; then
|
||||
# shellcheck disable=SC2086
|
||||
if SG="$(semgrep --config p/security-audit --config p/secrets --json --metrics=off $SCOPE_PATHS 2>/dev/null)"; then
|
||||
if SG="$(semgrep --config p/security-audit --config p/secrets --config p/javascript --json --metrics=off $SCOPE_PATHS 2>/dev/null)"; then
|
||||
NORM="$(echo "$SG" | jq '[.results[] | {
|
||||
id: ("semgrep-" + (.check_id|split(".")|last) + "-" + (.start.line|tostring)),
|
||||
title: ((.check_id|split(".")|last) + ": " + ((.extra.message // "")[0:120])),
|
||||
|
|
@ -79,15 +79,24 @@ if command -v semgrep >/dev/null; then
|
|||
else echo " [semgrep] run failed" >&2; fi
|
||||
else note_missing semgrep "brew install semgrep"; fi
|
||||
|
||||
# --- gitleaks (hardcoded secrets) ---
|
||||
# --- gitleaks (hardcoded secrets) — git-mode respects .gitignore (skips gitignored .env etc.) ---
|
||||
if command -v gitleaks >/dev/null; then
|
||||
GLALL="$TMP/gl.json"; echo '[]' > "$GLALL"
|
||||
for p in $SCOPE_PATHS; do
|
||||
gitleaks dir "$p" --report-format json --report-path "$TMP/gl1.json" >/dev/null 2>&1 || true
|
||||
if git -C "$TARGET" rev-parse --is-inside-work-tree >/dev/null 2>&1; then
|
||||
# Scan committed content at the repo root; gitignored files (e.g. a local .env with real
|
||||
# keys) are excluded by design, so the gate never false-blocks on them. Same JSON schema.
|
||||
gitleaks git "$TARGET" --report-format json --report-path "$TMP/gl1.json" >/dev/null 2>&1 || true
|
||||
if [ -s "$TMP/gl1.json" ]; then
|
||||
jq -s '.[0]+(.[1] // [])' "$GLALL" "$TMP/gl1.json" > "$GLALL.t" && mv "$GLALL.t" "$GLALL"; rm -f "$TMP/gl1.json"
|
||||
fi
|
||||
done
|
||||
else
|
||||
for p in $SCOPE_PATHS; do
|
||||
gitleaks dir "$p" --report-format json --report-path "$TMP/gl1.json" >/dev/null 2>&1 || true
|
||||
if [ -s "$TMP/gl1.json" ]; then
|
||||
jq -s '.[0]+(.[1] // [])' "$GLALL" "$TMP/gl1.json" > "$GLALL.t" && mv "$GLALL.t" "$GLALL"; rm -f "$TMP/gl1.json"
|
||||
fi
|
||||
done
|
||||
fi
|
||||
NORM="$(jq '[.[] | {
|
||||
id: ("gitleaks-" + .RuleID + "-" + (.StartLine|tostring)),
|
||||
title: ("secret: " + .Description), severity: "high", cwe: "CWE-798",
|
||||
|
|
|
|||
93
security-review/skill/sh-security-review.md
Normal file
93
security-review/skill/sh-security-review.md
Normal file
|
|
@ -0,0 +1,93 @@
|
|||
---
|
||||
name: sh-security-review
|
||||
description: High-recall agentic security review with anti-complacency structure. Opus fans out N narrow fresh-context detectors over the target, a separate fresh-context verifier demands proof-of-exploit or downgrades to unverified, then findings are emitted in the structured schema with a block decision. Returns severity-ranked findings each carrying a concrete proof.
|
||||
---
|
||||
|
||||
# Sea Haven Security Review (detector fan-out + proof-or-kill verifier)
|
||||
|
||||
A high-recall security review built to resist reviewer complacency. Instead of one model judging the
|
||||
whole surface (that's `sh-build-review`), this fans out **N narrow detectors, each fresh context and
|
||||
adversarial**, then a **separate verifier** with the opposing incentive demands a concrete
|
||||
proof-of-exploit for every candidate or downgrades it to `unverified`. Output is the structured
|
||||
finding schema (`~/.claude/security-review/finding.schema.json`) plus a block decision.
|
||||
|
||||
This is the interactive (Path A) entry point and runs under the Max subscription. The same prompts and
|
||||
schema are reused by the automated `review.sh` (Path B) later. See memory `project-security-review-agent`.
|
||||
|
||||
## When to Use
|
||||
- Security review of a branch, working tree, file, or whole repo
|
||||
- Before pushing payments/auth/IaC/input-handling changes
|
||||
- As the high-recall pass; complements `/security-review`, `/code-review ultra`, and `sh-build-review`
|
||||
|
||||
## Arguments
|
||||
- Optional target: a path, file, branch, or diff. Default: the current working tree / branch diff vs main.
|
||||
- Optional `--scope <dirs>`: restrict the scan (e.g. `src/ infra/ web/`).
|
||||
|
||||
## Mechanism (Opus runs these)
|
||||
|
||||
Opus is the main loop. It scopes the target, runs the detector fan-out and verifier as subagents
|
||||
(each with **fresh context** so no "we already passed 10 files" approval prior accumulates), then
|
||||
applies the gate rule and reports. Opus does NOT soften the gate; the block condition is mechanical.
|
||||
|
||||
### 1. Scope
|
||||
Identify the files in scope (respect `--scope`; never scan an answer key or `.git`). Note the languages
|
||||
present (Python/Lambda, .NET, React/JS, SAM/CDK IaC) so detectors apply the right checklist.
|
||||
|
||||
### 2. Detector fan-out (parallel, fresh context, closed checklist, adversarial)
|
||||
Spawn these detectors as **separate parallel subagents** (`subagent_type: general-purpose`). Each gets
|
||||
ONLY its category, the schema, and the adversarial framing. Each must, per checklist item, either cite a
|
||||
specific safe line OR file a finding — no open "looks fine" judgment.
|
||||
|
||||
Detectors:
|
||||
- **injection** — SQL/command/template injection, unsafe deserialization, SSRF, path/file traversal, XXE
|
||||
- **authz** — broken object-level auth/IDOR, missing access checks, missing webhook/Slack signature verification, auth bypass
|
||||
- **secrets-crypto** — hardcoded secrets/keys, weak/again crypto (MD5/SHA1/unsalted), sensitive data in logs/errors/responses, wrong SSM-vs-Secrets-Manager
|
||||
- **iac-iam** — wildcard IAM actions/resources, public buckets/endpoints, open security-group ingress (0.0.0.0/0), missing encryption, over-broad trust policies
|
||||
- **web-client** — XSS (incl. dangerouslySetInnerHTML), CSRF, open redirect, client-side secret exposure
|
||||
- **logic** — broken multi-step invariants, race conditions, missing tenant isolation, auth-state confusion
|
||||
|
||||
Detector prompt template (fill `{CATEGORY}`, `{CHECKLIST}`, `{SCOPE}`):
|
||||
```
|
||||
You are a hostile {CATEGORY} security auditor for Sea Haven. Assume this code is hostile and the author
|
||||
missed something. Audit ONLY {CATEGORY} issues in: {SCOPE}. Read the actual files.
|
||||
For each checklist item, either name a specific line that is safe, OR file a finding. Do not hand-wave.
|
||||
Checklist: {CHECKLIST}
|
||||
Return a JSON array of findings, each: {id, title, claimed_severity (critical|high|medium|low|info),
|
||||
cwe (CWE-####), file, line, category:"{CATEGORY}", data_flow (numbered source->sink trace),
|
||||
proof:{input, outcome, test|null}, recommendation}. If none, return []. No prose outside the JSON.
|
||||
```
|
||||
|
||||
### 3. Verifier (separate subagent, fresh context, proof-or-kill)
|
||||
Spawn one or more **verifier** subagents with the OPPOSING incentive. The verifier did not find these and
|
||||
is rewarded for killing weak claims. For each candidate it demands a concrete, plausible proof.
|
||||
```
|
||||
You are a skeptical exploitation verifier. You did NOT find these; your job is to REFUTE weak claims.
|
||||
For each candidate finding, decide: is there a concrete, plausible proof-of-exploit (a specific malicious
|
||||
input and the specific bad outcome, consistent with the code)?
|
||||
- If yes: status="confirmed", keep severity = claimed_severity, tighten the proof.
|
||||
- If no / speculative / not reachable: status="unverified" and severity="unverified". Default to unverified
|
||||
when uncertain. A confident assertion without a demonstrable input is NOT proof.
|
||||
Return the findings array with status, severity, and the verified proof. No prose outside the JSON.
|
||||
```
|
||||
|
||||
### 4. Merge + gate (mechanical, Opus does not soften)
|
||||
- Dedup by (file, cwe, nearby line); keep the highest accepted severity.
|
||||
- `summary.confirmed_critical` / `confirmed_high` = counts of status=confirmed at that severity.
|
||||
- `summary.block = true` if any unsuppressed confirmed critical/high exists.
|
||||
- A finding may be suppressed ONLY with a written `suppression_justification`, which is surfaced in the report.
|
||||
No silent dismissal: a critical/high with no proof becomes `unverified` (still listed), never deleted.
|
||||
|
||||
### 5. Report
|
||||
Emit the schema JSON, then a human summary: BLOCK/PASS, confirmed findings highest-severity-first, each
|
||||
with file:line, the numbered data-flow trace, and the proof. List unverified and suppressed separately so
|
||||
nothing is silently dropped. The reader reviews proofs, not raw code.
|
||||
|
||||
## Relationship to existing tooling
|
||||
- `sh-build-review` — single deep Fable pass over a change surface (depth, one reasoner).
|
||||
- `sh-security-review` — high-recall fan-out + proof-or-kill verifier (breadth + anti-complacency).
|
||||
- IAM/policy and Lambda-signature changes still require the mandatory cross-family review per global instructions; this does not replace it.
|
||||
|
||||
## Output
|
||||
- Structured findings (schema) + block decision
|
||||
- Confirmed findings with proofs, unverified and suppressed listed separately
|
||||
- HARD STOP / BLOCK surfaced when `summary.block` is true
|
||||
Reference in a new issue