mirror of
https://github.com/Sea-Haven-Industries/security-review.git
synced 2026-09-30 04:33:13 +00:00
feat(scanner): auto-resolve + merge suppressions when --suppressions absent
review.sh only applied suppressions when handed an explicit --suppressions
FILE, so only the pre-push hook resolved them. Every other entry point (the
Open SWE daily-report automation, nightly sweep, on-demand/CI, agent runs)
called review.sh without it and therefore suppressed nothing, re-surfacing
every already-adjudicated false positive as HIGH.
When --suppressions is not passed, resolve by repo basename and MERGE both
suppression locations (machine-level first, wins id collisions):
- machine-level: ${SH_SECURITY_SUPPRESSIONS_DIR:-~/.config/sea-haven/security-review}/<basename>/suppressions.json
- repo-local: <repo>/.security-review/suppressions.json
An explicit --suppressions still overrides, so the hook and existing callers
are unaffected. Degrades gracefully off-Mac (repo-local only); fail-safe on an
unparseable file (suppresses nothing → blocks).
SECURITY (/sh-security-review, 2026-07-13): fan-out + proof-or-kill confirmed
one HIGH — the git-tracked repo-local suppressions.json lets anyone who can
commit to a scanned repo suppress a real finding and PASS an automated run
(verified by an actual exploit run; same posture nightly_sweep already had).
ACCEPTED-RISK per Adam on the condition that the automated scanners only ever
target trusted repos (no unreviewed untrusted contributions). Documented in the
auto-resolve block, README trust-model note, and a hard warning in
sweep-targets.txt. Four other candidates downgraded to low/pre-existing.
Verified: machine-level and repo-local both auto-resolve and suppress; explicit
--suppressions override still blocks; simulated off-Mac host keeps repo-local
and correctly re-blocks machine-level-only FPs.
This commit is contained in:
parent
9cac679bc3
commit
f0492b7555
3 changed files with 69 additions and 3 deletions
22
README.md
22
README.md
|
|
@ -40,13 +40,29 @@ The global mode sets `git config --global core.hooksPath ~/.config/git/hooks`. S
|
|||
its own local `core.hooksPath` overrides the global hook — install per-repo there. See memory
|
||||
`reference_global_security_review_hook`.
|
||||
|
||||
**Suppressing a false positive.** A written justification is required and is surfaced in the report. The
|
||||
hooks resolve a suppressions file in this order:
|
||||
**Suppressing a false positive.** A written justification is required and is surfaced in the report.
|
||||
When no `--suppressions FILE` is passed, `review.sh` auto-resolves and **merges** suppressions from
|
||||
two locations (an explicit `--suppressions` still overrides both):
|
||||
1. **Machine-level (preferred), kept out of repo history:**
|
||||
`${SH_SECURITY_SUPPRESSIONS_DIR:-~/.config/sea-haven/security-review}/<repo-basename>/suppressions.json`
|
||||
(override the base dir with `SH_SECURITY_SUPPRESSIONS_DIR`). Keeps a suppression from becoming a
|
||||
permanent in-history "ignore."
|
||||
2. **Repo-local fallback:** `<repo>/.security-review/suppressions.json` (used only if no machine-level file exists).
|
||||
2. **Repo-local:** `<repo>/.security-review/suppressions.json` (tracked; travels with the repo).
|
||||
|
||||
Both files' `.suppressions[]` are concatenated (machine-level first, so it wins any id collision).
|
||||
This means every entry point — the pre-push hook, the nightly sweep, on-demand/CI runs, and the
|
||||
Open SWE daily-report automation — resolves suppressions identically. **Portability note:** hosts that
|
||||
cannot see the Mac's `~/.config` (the Open SWE automation, the R720 VM) get only the tracked repo-local
|
||||
file, so a suppression that must be honored off-Mac has to live repo-local. Fail-safe: if a file is
|
||||
present but unparseable, nothing is suppressed (the gate blocks).
|
||||
|
||||
> **⚠️ Trust model — automated scanners must target TRUSTED repos only.** The repo-local
|
||||
> `.security-review/suppressions.json` is git-tracked, so anyone who can land a commit in a scanned
|
||||
> repo can suppress a real finding by committing it alongside the vuln (scanner IDs are deterministic).
|
||||
> `/sh-security-review` confirmed this as a HIGH gate-bypass; it is **accepted** on the condition that
|
||||
> the automated scanners (Open SWE daily report, nightly sweep) only ever scan repos that do **not**
|
||||
> merge untrusted contributions without human review. See `sweep-targets.txt`. Relaxing that scoping
|
||||
> requires a trust check first (signed / CODEOWNERS-verified repo-local suppressions).
|
||||
|
||||
Same JSON either place: `{"suppressions":[{"id":"<review.sh finding id>","justification":"…"}]}`. Caveat:
|
||||
machine-level files are keyed by **repo basename**, so two repos sharing a name collide — fine for the
|
||||
|
|
|
|||
39
review.sh
39
review.sh
|
|
@ -32,6 +32,45 @@ TMP="$(mktemp -d)"; trap 'rm -rf "$TMP"' EXIT
|
|||
ALL="$TMP/all.json"; echo '[]' > "$ALL"
|
||||
add() { jq --argjson add "$1" '. + $add' "$ALL" > "$ALL.t" && mv "$ALL.t" "$ALL"; }
|
||||
|
||||
# --- Auto-resolve suppressions when --suppressions was not passed ---
|
||||
# Mirrors the pre-push hook's resolution, but MERGES machine-level + repo-local
|
||||
# (the hook uses XOR: first match wins) and degrades gracefully. On hosts without
|
||||
# the machine-level dir — the Open SWE daily-report automation and the R720 VM,
|
||||
# which clone repos but cannot see ~/.config on Adam's Mac — only the tracked
|
||||
# repo-local file is used, so suppressions travel inside the cloned repo. On the
|
||||
# Mac both files are merged. An explicit --suppressions FILE still overrides this,
|
||||
# so the hook and any existing caller are unaffected. Fail-safe: if resolution or
|
||||
# the merge fails, SUPPRESSIONS stays empty and the gate suppresses nothing (blocks).
|
||||
#
|
||||
# SECURITY — ACCEPTED RISK (Adam, 2026-07-13, /sh-security-review confirmed HIGH):
|
||||
# The repo-local .security-review/suppressions.json is GIT-TRACKED, so anyone who
|
||||
# can land a commit in a scanned repo can ship a real finding together with a
|
||||
# suppression for it (scanner IDs are deterministic/precomputable) and make an
|
||||
# AUTOMATED bare-review.sh run PASS. This is the same trust posture nightly_sweep
|
||||
# already used. It is ACCEPTED on the condition that the automated scanners
|
||||
# (Open SWE daily report, nightly sweep) only ever target TRUSTED repos — no repo
|
||||
# that merges untrusted contributions without human review may be added to their
|
||||
# target lists. See sweep-targets.txt and memory reference_global_security_review_hook.
|
||||
# Do NOT relax that scoping without adding a trust check (signed/CODEOWNERS-verified
|
||||
# repo-local suppressions).
|
||||
if [ -z "$SUPPRESSIONS" ]; then
|
||||
_repo_root="$(git -C "$TARGET" rev-parse --show-toplevel 2>/dev/null || echo "$TARGET")"
|
||||
_machine_sup="${SH_SECURITY_SUPPRESSIONS_DIR:-$HOME/.config/sea-haven/security-review}/$(basename "$_repo_root")/suppressions.json"
|
||||
_repo_sup="$_repo_root/.security-review/suppressions.json"
|
||||
_sup_files=()
|
||||
[ -f "$_machine_sup" ] && _sup_files+=("$_machine_sup") # machine-level first → wins id collisions
|
||||
[ -f "$_repo_sup" ] && _sup_files+=("$_repo_sup")
|
||||
if [ "${#_sup_files[@]}" -gt 0 ]; then
|
||||
_merged="$TMP/suppressions.merged.json"
|
||||
if jq -s '{suppressions: (map(.suppressions // []) | add)}' "${_sup_files[@]}" > "$_merged" 2>/dev/null; then
|
||||
SUPPRESSIONS="$_merged"
|
||||
echo "== Suppressions auto-resolved: ${_sup_files[*]} ==" >&2
|
||||
else
|
||||
echo " [WARN] suppression file(s) present but unparseable; suppressing nothing: ${_sup_files[*]}" >&2
|
||||
fi
|
||||
fi
|
||||
fi
|
||||
|
||||
# Map a scope list into find paths under TARGET (default: whole target).
|
||||
scope_paths() {
|
||||
if [ -n "$SCOPE" ]; then for d in $SCOPE; do [ -e "$TARGET/$d" ] && echo "$TARGET/$d"; done
|
||||
|
|
|
|||
|
|
@ -2,6 +2,17 @@
|
|||
# Lines starting with '#' and blank lines are ignored. ~ is expanded.
|
||||
# Override at runtime with the TARGETS env var (space-separated paths).
|
||||
#
|
||||
# ┌─ SECURITY — TRUSTED REPOS ONLY (accepted-risk condition, 2026-07-13) ─────────┐
|
||||
# │ review.sh auto-loads each scanned repo's GIT-TRACKED │
|
||||
# │ .security-review/suppressions.json. A contributor who can land a commit can │
|
||||
# │ therefore suppress a real finding (ship the vuln + its own suppression) and │
|
||||
# │ make the automated run PASS — /sh-security-review confirmed this as HIGH and │
|
||||
# │ it was ACCEPTED only on the condition that this list (and any target list the │
|
||||
# │ Open SWE daily-report automation uses) contains ONLY trusted repos. Do NOT │
|
||||
# │ add a repo that merges untrusted contributions without human review. The same │
|
||||
# │ rule applies to the Open SWE daily automation's target set — keep it in sync. │
|
||||
# └───────────────────────────────────────────────────────────────────────────────┘
|
||||
#
|
||||
# NOTE: the testbed canary corpus is ALWAYS scanned by nightly_sweep.sh as the
|
||||
# anti-complacency check; do NOT list it here (it is handled separately).
|
||||
#
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue