Skip to content

Commit 0081de9

Browse files
solomonneascodex
solomonneas
and
codex
authored
fix(guard): scope new-branch history scans to remote refs (#606)
Co-authored-by: Codex <codex@openai.com>
1 parent d6f828d commit 0081de9

4 files changed

Lines changed: 894 additions & 20 deletions

File tree

‎src/brigade/guard/git_scan.py‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22

33
import argparse
44
import json
5+
import re
56
import subprocess
67
import sys
78
from pathlib import Path
89

910
from .engine import scan_text
10-
from .policy import Policy, load_policy
11+
from .policy import Policy, default_policy, load_policy
1112
from .report import to_text
1213

1314

@@ -37,12 +38,28 @@ def main(argv: list[str] | None = None) -> int:
3738
action="store_true",
3839
help="scan content INTRODUCED across commit history (added lines), not just the current tip",
3940
)
40-
parser.add_argument("--range", dest="rev_range", help="revision range for --history, e.g. origin/main..HEAD")
41-
parser.add_argument("--all", action="store_true", help="with --history, scan all reachable commits")
41+
history_source = parser.add_mutually_exclusive_group()
42+
history_source.add_argument(
43+
"--range", dest="rev_range", help="revision range for --history, e.g. origin/main..HEAD"
44+
)
45+
history_source.add_argument("--all", action="store_true", help="with --history, scan all reachable commits")
46+
history_source.add_argument(
47+
"--revs-stdin",
48+
action="store_true",
49+
help=(
50+
"with --history, read the exact set of commit revisions to scan from "
51+
"stdin (one full 40- or 64-character hex SHA per line) instead of "
52+
"calling git rev-list. Lets a caller batch a precomputed revision "
53+
"set into one scan process and keeps revisions off argv (no ARG_MAX "
54+
"limit, no option injection)."
55+
),
56+
)
4257
parser.add_argument("--json", action="store_true", help="emit JSON report")
4358
args = parser.parse_args(argv)
59+
if args.revs_stdin and not args.history:
60+
parser.error("--revs-stdin requires --history")
4461

45-
policy = load_policy(args.policy) if args.policy else _default_repo_policy()
62+
policy = _load_repo_policy(args.policy)
4663
_merge_allow_values_from(policy, args.allow_values_from or [])
4764

4865
if args.history:
@@ -146,6 +163,20 @@ def _scan_history(policy: Policy, args: argparse.Namespace) -> int:
146163

147164

148165
def _history_revs(args: argparse.Namespace) -> list[str]:
166+
if getattr(args, "revs_stdin", False):
167+
revs: list[str] = []
168+
for line_number, line in enumerate(sys.stdin, start=1):
169+
rev = line.strip()
170+
if not rev:
171+
continue
172+
if re.fullmatch(r"(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})", rev) is None:
173+
print(
174+
f"invalid revision on stdin line {line_number}: expected a full 40- or 64-character hex SHA",
175+
file=sys.stderr,
176+
)
177+
raise SystemExit(2)
178+
revs.append(rev.lower())
179+
return revs
149180
if args.all:
150181
cmd = ["git", "rev-list", "--all"]
151182
elif args.rev_range:
@@ -167,7 +198,9 @@ def _added_lines(rev: str) -> str:
167198
check=False,
168199
)
169200
if proc.returncode != 0:
170-
return ""
201+
detail = (proc.stderr or "git show failed").strip()
202+
print(f"cannot read commit {rev}: {detail}", file=sys.stderr)
203+
raise SystemExit(2)
171204
out = []
172205
for line in proc.stdout.splitlines():
173206
if line.startswith("+") and not line.startswith("+++"):
@@ -219,6 +252,16 @@ def _default_repo_policy() -> Policy:
219252
)
220253

221254

255+
def _load_repo_policy(raw: str | None) -> Policy:
256+
if raw is None:
257+
return _default_repo_policy()
258+
path = Path(raw)
259+
if path.is_file() or path.parent != Path("."):
260+
return load_policy(path)
261+
name = path.name if path.suffix == ".json" else f"{path.name}.json"
262+
return load_policy(default_policy(name))
263+
264+
222265
def _tracked_paths(*, all_tracked: bool, cwd: Path | None = None) -> list[Path]:
223266
if all_tracked:
224267
cmd = ["git", "ls-files"]

‎src/brigade/templates/hooks/pre-push‎

Lines changed: 175 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,37 @@
99
#
1010
# Uses Brigade's embedded content guard. CONTENT_GUARD_DIR is an explicit
1111
# compatibility override for an older standalone checkout.
12+
#
13+
# Two scans run per policy:
14+
# 1. tracked tip - working tree files (brigade scrub / content_guard scan)
15+
# 2. push history - content INTRODUCED by the commits being pushed
16+
# (closes the forward-scrub gap: a clean tip can still
17+
# sit on top of commits that leak in their diffs)
18+
#
19+
# For brand-new branches (remote SHA all-zero) the history scan derives the
20+
# exclusion set from `git ls-remote <url>` rather than local tracking refs,
21+
# which can be stale. Only commits not reachable from any advertised ref are
22+
# scanned, batched into one guard history process per policy via --revs-stdin.
1223
set -euo pipefail
1324

1425
REPO_ROOT="$(git rev-parse --show-toplevel)"
26+
cd "$REPO_ROOT"
27+
28+
# Git invokes pre-push as: pre-push <remote-name> <url>. The URL is the
29+
# destination actually being pushed to; it is the only source of truth for
30+
# what the remote currently advertises. Local remote-tracking refs
31+
# (refs/remotes/<name>/*) are NOT used to derive the exclusion set, because
32+
# they can be stale: a tracking ref can point at a commit the remote no
33+
# longer holds, or be absent for a commit the remote already holds. Using
34+
# them would over- or under-exclude and either miss a real leak or block on
35+
# already-public content. ${2:-} keeps `set -u` happy when a caller forgets
36+
# the URL (git always passes both).
37+
REMOTE_URL="${2:-}"
38+
1539
rc=0
40+
PLUMBING_ERR=0
41+
STDIN_REFS="$(cat)"
42+
1643
if [[ -n "${CONTENT_GUARD_DIR:-}" ]]; then
1744
POLICY="${CONTENT_GUARD_POLICY:-$CONTENT_GUARD_DIR/policies/public-repo.json}"
1845
if [[ ! -d "$CONTENT_GUARD_DIR" ]]; then
@@ -24,32 +51,169 @@ if [[ -n "${CONTENT_GUARD_DIR:-}" ]]; then
2451
exit 1
2552
fi
2653
echo "pre-push: scanning $REPO_ROOT with explicit content-guard compatibility override"
27-
PYTHONPATH="$CONTENT_GUARD_DIR/src" python3 -m content_guard scan "$REPO_ROOT" --policy "$POLICY" || rc=$?
54+
RUN_TIP() {
55+
local pol="$1"
56+
PYTHONPATH="$CONTENT_GUARD_DIR/src" python3 -m content_guard scan "$REPO_ROOT" --policy "$pol" || rc=$?
57+
}
58+
RUN_HISTORY() {
59+
local pol="$1"
60+
PYTHONPATH="$CONTENT_GUARD_DIR/src" python3 -m content_guard.git_scan \
61+
--history --revs-stdin --policy "$pol" "${ALLOW_FROM[@]+"${ALLOW_FROM[@]}"}" || rc=$?
62+
}
63+
RUN_HISTORY_RANGE() {
64+
local range="$1"
65+
local pol="$2"
66+
PYTHONPATH="$CONTENT_GUARD_DIR/src" python3 -m content_guard.git_scan \
67+
--history --range "$range" --policy "$pol" "${ALLOW_FROM[@]+"${ALLOW_FROM[@]}"}" || rc=$?
68+
}
2869
else
2970
POLICY="${CONTENT_GUARD_POLICY:-public-repo}"
3071
if ! command -v brigade >/dev/null 2>&1; then
3172
echo "pre-push: brigade not found; reinstall brigade-cli to restore the embedded guard" >&2
3273
exit 1
3374
fi
3475
echo "pre-push: scanning $REPO_ROOT with Brigade's embedded guard"
35-
brigade scrub --target "$REPO_ROOT" --policy "$POLICY" --no-receipt || rc=$?
76+
RUN_TIP() {
77+
local pol="$1"
78+
brigade scrub --target "$REPO_ROOT" --policy "$pol" --no-receipt || rc=$?
79+
}
80+
RUN_HISTORY() {
81+
local pol="$1"
82+
brigade guard git --history --revs-stdin --policy "$pol" "${ALLOW_FROM[@]+"${ALLOW_FROM[@]}"}" || rc=$?
83+
}
84+
RUN_HISTORY_RANGE() {
85+
local range="$1"
86+
local pol="$2"
87+
brigade guard git --history --range "$range" --policy "$pol" "${ALLOW_FROM[@]+"${ALLOW_FROM[@]}"}" || rc=$?
88+
}
89+
fi
90+
91+
# Optional private policy (a private identifier denylist kept out of public
92+
# repos). If present, the hook runs a second scan pass against it.
93+
EXTRA_POLICY="${CONTENT_GUARD_EXTRA_POLICY:-$HOME/.config/content-guard/internal.json}"
94+
ALLOW_FROM=()
95+
[[ -f "$EXTRA_POLICY" ]] && ALLOW_FROM=(--allow-values-from "$EXTRA_POLICY")
96+
97+
# Track the worst scanner outcome across all scans: BLOCKED (rc=1, a real
98+
# leak verdict) vs SCANNER_ERR (rc>1, the scanner failed to run). Issue #82:
99+
# never mislabel a scanner/plumbing failure as found violations.
100+
BLOCKED=0
101+
SCANNER_ERR=0
102+
note_rc() {
103+
if (( rc > 1 )); then SCANNER_ERR=$rc; elif (( rc == 1 )); then BLOCKED=1; fi
104+
rc=0
105+
}
106+
107+
ADVERTISED_REFS=""
108+
REMOTE_ENUMERATED=0
109+
110+
prepare_new_branch_ranges() {
111+
local needs_remote=0
112+
local _lref lsha _rref rsha
113+
while read -r _lref lsha _rref rsha; do
114+
[[ -z "${lsha:-}" || "$lsha" =~ ^0+$ ]] && continue
115+
if [[ "$rsha" =~ ^0+$ ]]; then
116+
needs_remote=1
117+
break
118+
fi
119+
done < <(printf '%s\n' "$STDIN_REFS")
120+
[[ "$needs_remote" -eq 1 ]] || return 0
121+
122+
if [[ -z "$REMOTE_URL" ]]; then
123+
echo "pre-push: [history] cannot enumerate remote refs (no destination URL)" >&2
124+
PLUMBING_ERR=1
125+
return
126+
fi
127+
128+
local advertised
129+
# Keep peeled annotated-tag lines (`refs/tags/<name>^{}`), which --refs
130+
# suppresses. The peeled commit can be present locally even when the tag
131+
# object is not, and it still marks that commit as already public.
132+
if ! advertised="$(GIT_TERMINAL_PROMPT=0 git ls-remote "$REMOTE_URL" 2>&1)"; then
133+
echo "pre-push: [history] failed to enumerate advertised refs for $REMOTE_URL" >&2
134+
while IFS= read -r line; do
135+
[[ -n "$line" ]] && echo "pre-push: $line" >&2
136+
done <<<"$advertised"
137+
PLUMBING_ERR=1
138+
return
139+
fi
140+
ADVERTISED_REFS="$advertised"
141+
REMOTE_ENUMERATED=1
142+
}
143+
144+
scan_with() {
145+
local pol="$1"
146+
local new_revs new_count range
147+
RUN_TIP "$pol"
148+
note_rc
149+
while read -r _lref lsha _rref rsha; do
150+
[[ -z "${lsha:-}" || "$lsha" =~ ^0+$ ]] && continue # branch deletion
151+
if [[ "$rsha" =~ ^0+$ ]]; then
152+
[[ "$REMOTE_ENUMERATED" -eq 1 ]] || continue
153+
if ! git rev-parse -q --verify "${lsha}^{commit}" >/dev/null; then
154+
echo "pre-push: [history] local commit $lsha does not resolve" >&2
155+
PLUMBING_ERR=1
156+
continue
157+
fi
158+
if ! new_revs="$(
159+
printf '%s\n' "$ADVERTISED_REFS" |
160+
awk 'NF { print "^" $1 }' |
161+
git rev-list --ignore-missing "$lsha" --stdin
162+
)"; then
163+
echo "pre-push: [history] failed to resolve local history for $lsha" >&2
164+
PLUMBING_ERR=1
165+
continue
166+
fi
167+
if [[ -z "$new_revs" ]]; then
168+
echo "pre-push: [history] no new commits to scan for $lsha"
169+
continue
170+
fi
171+
new_count="$(printf '%s\n' "$new_revs" | grep -c .)"
172+
echo "pre-push: [history] scanning introduced content in $new_count new commit(s) for ${_lref:-$lsha}"
173+
# Batch the exact new-commit set into ONE guard history process via
174+
# --revs-stdin. Revisions are data on stdin, not argv: no shell
175+
# splitting, no option injection, one process per policy regardless
176+
# of how many commits are new.
177+
RUN_HISTORY "$pol" <<<"$new_revs"
178+
note_rc
179+
else
180+
range="$rsha..$lsha"
181+
echo "pre-push: [history] scanning introduced content in $range"
182+
RUN_HISTORY_RANGE "$range" "$pol"
183+
note_rc
184+
fi
185+
done < <(printf '%s\n' "$STDIN_REFS")
186+
}
187+
188+
prepare_new_branch_ranges
189+
scan_with "$POLICY"
190+
if [[ -n "$EXTRA_POLICY" && -f "$EXTRA_POLICY" ]]; then
191+
scan_with "$EXTRA_POLICY"
36192
fi
37193

38-
if [[ "$rc" -eq 0 ]]; then
39-
exit 0
194+
if (( SCANNER_ERR > 0 )); then
195+
# Any non-1 exit code is the scanner failing to run (missing deps, bad
196+
# policy, crash, or a plumbing failure surfaced as exit 2), not a leak
197+
# verdict. Do not mislabel it as found violations (issue #82).
198+
echo >&2
199+
echo "pre-push: content-guard failed to run (exit code $SCANNER_ERR); this is a scanner error, not a leak verdict." >&2
200+
echo "pre-push: re-run it directly to see the error, then push again once the scanner works." >&2
201+
exit 1
202+
fi
203+
204+
if (( PLUMBING_ERR > 0 )); then
205+
echo >&2
206+
echo "pre-push: remote history could not be determined; this is a git plumbing error, not a leak verdict." >&2
207+
echo "pre-push: fix the remote or local Git state, then push again." >&2
208+
exit 1
40209
fi
41210

42-
if [[ "$rc" -eq 1 ]]; then
211+
if [[ "$BLOCKED" -ne 0 ]]; then
43212
echo >&2
44213
echo "pre-push: BLOCKED. content-guard found violations." >&2
45214
echo "pre-push: fix the leak, or add an inline allow-tag on the offending line:" >&2
46215
echo "pre-push: <!-- content-guard: allow <rule-id> -->" >&2
47216
exit 1
48217
fi
49218

50-
# Any other exit code is the scanner failing to run (missing deps, bad policy,
51-
# crash), not a leak verdict. Do not mislabel it as found violations.
52-
echo >&2
53-
echo "pre-push: content-guard failed to run (exit code $rc); this is a scanner error, not a leak verdict." >&2
54-
echo "pre-push: re-run it directly to see the error, then push again once the scanner works." >&2
55-
exit 1
219+
exit 0

0 commit comments

Comments
 (0)