diff --git a/security-review/finding.schema.json b/security-review/finding.schema.json new file mode 100644 index 0000000..52be56e --- /dev/null +++ b/security-review/finding.schema.json @@ -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)" } + } + } + } +} diff --git a/security-review/hooks/pre-commit b/security-review/hooks/pre-commit index 242c568..c28b1f6 100755 --- a/security-review/hooks/pre-commit +++ b/security-review/hooks/pre-commit @@ -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" diff --git a/security-review/hooks/pre-push b/security-review/hooks/pre-push new file mode 100755 index 0000000..74118d8 --- /dev/null +++ b/security-review/hooks/pre-push @@ -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 diff --git a/security-review/install-hooks.sh b/security-review/install-hooks.sh index 87b7668..6a10a78 100755 --- a/security-review/install-hooks.sh +++ b/security-review/install-hooks.sh @@ -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 /.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 + local src="$1" dest="$2" + cp "$src" "$dest" + chmod +x "$dest" +} + +link_asset() { # link_asset (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 ' 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 diff --git a/security-review/review.sh b/security-review/review.sh index eb713f2..af5f766 100755 --- a/security-review/review.sh +++ b/security-review/review.sh @@ -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", diff --git a/security-review/skill/sh-security-review.md b/security-review/skill/sh-security-review.md new file mode 100644 index 0000000..96dcc45 --- /dev/null +++ b/security-review/skill/sh-security-review.md @@ -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 `: 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