Skip to content

Commit 45d8c2c

Browse files
1 parent 661f2d3 commit 45d8c2c

5 files changed

Lines changed: 105 additions & 41 deletions

File tree

‎.github/security/ai_security_review.py‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -312,13 +312,21 @@ def clip(text: str, keep_around: str = "", limit: int = MAX_LINE_CHARS) -> str:
312312
return text[:limit] + "…"
313313

314314

315-
def build_evidence(repo: str, token: str, findings: list[dict]):
315+
def build_evidence(repo: str, token: str, findings: list[dict],
316+
mask_also: list[dict] = ()):
316317
"""Turn scanner findings into redacted windows. Returns (evidence, errors).
317318
318-
Any error fails the gate: an unreadable window is an unjudged detection.
319+
`mask_also` holds detections past the triage cap: they get no window, but
320+
are blanked wherever they fall inside one. Any error fails the gate: an
321+
unreadable window is an unjudged detection.
319322
"""
320323
evidence, errors = [], []
321324

325+
masks: dict[tuple[str, str], dict[int, dict]] = {}
326+
for f in [*findings, *mask_also]:
327+
if f["location_kind"] != "archive":
328+
masks.setdefault((f["blob_sha"], f["file"]), {})[f["id"]] = f
329+
322330
# Grouped by (blob, path) so identical files each get their own markers;
323331
# fetched once per blob.
324332
by_path: dict[tuple[str, str], list[dict]] = {}
@@ -331,7 +339,7 @@ def build_evidence(repo: str, token: str, findings: list[dict]):
331339
by_path.setdefault((f["blob_sha"], f["file"]), []).append(f)
332340

333341
blobs: dict[str, tuple[bytes | None, str]] = {}
334-
for (blob_sha, _path), group in by_path.items():
342+
for (blob_sha, path), group in by_path.items():
335343
if blob_sha not in blobs:
336344
blobs[blob_sha] = fetch_blob(repo, blob_sha, token, MAX_BLOB_BYTES)
337345
raw, reason = blobs[blob_sha]
@@ -343,8 +351,8 @@ def build_evidence(repo: str, token: str, findings: list[dict]):
343351
# Split on b"\n" only, to match the scanner's line numbers; decode
344352
# after masking.
345353
lines = raw.split(b"\n")
346-
masked = [m.decode("utf-8", "replace")
347-
for m in mask_detections(lines, group)]
354+
in_file = list(masks[(blob_sha, path)].values())
355+
masked = [m.decode("utf-8", "replace") for m in mask_detections(lines, in_file)]
348356

349357
for f in group:
350358
if f["location_kind"] == "path":
@@ -684,7 +692,8 @@ def render_comment(review, scan_conclusion, blocked, note=""):
684692
findings = review["findings"]
685693
if not findings:
686694
# No green tick under a red banner.
687-
clean = not (review["injection"] or review["unanswered"] or blocked)
695+
clean = not (review["injection"] or review["unanswered"] or blocked
696+
or scan_conclusion == "failure")
688697
L += ["✅ No secrets in the changed files." if clean
689698
else "No secrets among the detections that *were* triaged.", ""]
690699
for f in findings:
@@ -804,7 +813,7 @@ def main() -> int:
804813
else:
805814
# Runs even when `blocked`: the status stays red, but reviewers still
806815
# need verdicts on what was scanned.
807-
evidence, errors = build_evidence(repo, gh_token, findings)
816+
evidence, errors = build_evidence(repo, gh_token, findings, scan["findings"])
808817
if errors:
809818
blocked = blocked or f"{len(errors)} detection(s) could not be read"
810819
for e in errors[:10]:

‎.github/security/sanitize_findings.py‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,26 @@
1010
from __future__ import annotations
1111

1212
import argparse
13+
import hashlib
14+
import hmac
1315
import json
1416
import os
17+
import secrets
1518
import sys
1619

1720
from redaction import build_redaction_map, redact, safe_rule_name
1821

1922

20-
def fingerprint(secret: str) -> str:
21-
"""Length plus first/last character, so reviewers can correlate findings.
23+
# Per-run key, never stored, so a digest cannot be brute-forced the way a bare
24+
# hash of a short secret can.
25+
_RUN_KEY = secrets.token_bytes(32)
2226

23-
Not a hash: a hash of a short secret is brute-forceable.
24-
"""
27+
28+
def fingerprint(secret: str) -> str:
29+
"""Keyed digest, so reviewers can correlate findings within one report."""
2530
if not secret:
2631
return ""
27-
return f"len={len(secret)}:{secret[0]}…{secret[-1]}"
32+
return hmac.new(_RUN_KEY, secret.encode(), hashlib.sha256).hexdigest()[:12]
2833

2934

3035
def sanitize_gitleaks(findings: list) -> list[dict]:

‎.github/security/scan_head.py‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -135,22 +135,22 @@ def run_betterleaks(work: str, config: str, report: str) -> None:
135135
`report` must be absolute.
136136
"""
137137
# Empty per-run dir; the ignore-path flag otherwise defaults to the scan target.
138-
noignore = tempfile.mkdtemp(prefix="bl-noignore-")
139-
proc = subprocess.run(
140-
[
141-
"betterleaks", "git", ".",
142-
f"--config={config}",
143-
f"--gitleaks-ignore-path={noignore}",
144-
"--ignore-gitleaks-allow",
145-
"--report-format=json",
146-
f"--report-path={report}",
147-
# Findings exit 0, so any non-zero exit is a scanner error.
148-
"--exit-code=0",
149-
"--max-target-megabytes=25",
150-
"--no-banner",
151-
],
152-
cwd=work, capture_output=True, text=True,
153-
)
138+
with tempfile.TemporaryDirectory(prefix="bl-noignore-") as noignore:
139+
proc = subprocess.run(
140+
[
141+
"betterleaks", "git", ".",
142+
f"--config={config}",
143+
f"--gitleaks-ignore-path={noignore}",
144+
"--ignore-gitleaks-allow",
145+
"--report-format=json",
146+
f"--report-path={report}",
147+
# Findings exit 0, so any non-zero exit is a scanner error.
148+
"--exit-code=0",
149+
"--max-target-megabytes=25",
150+
"--no-banner",
151+
],
152+
cwd=work, capture_output=True, text=True,
153+
)
154154
sys.stderr.write(proc.stderr[-4000:])
155155
if proc.returncode != 0:
156156
raise RuntimeError(f"betterleaks exited {proc.returncode}")
@@ -299,11 +299,11 @@ def main() -> int:
299299
if altered:
300300
log(f"::error title=Files not scanned::{len(altered)} materialised file(s) "
301301
"did not reach the scan commit byte-identically")
302-
report_path = os.path.join(os.path.dirname(out_path) or ".", "betterleaks.raw.json")
303-
run_betterleaks(work, config, report_path)
302+
raw_report = os.path.join(os.path.dirname(out_path) or ".", "betterleaks.raw.json")
303+
run_betterleaks(work, config, raw_report)
304304

305305
blob_by_path = {f["path"]: f["blob_sha"] for f in written}
306-
raw = load_report(report_path)
306+
raw = load_report(raw_report)
307307
findings, unmappable = merge_findings(raw, blob_by_path, work)
308308
skipped.extend(unmappable)
309309
merged = len(raw) - len(findings) - len(unmappable)
@@ -314,7 +314,7 @@ def main() -> int:
314314

315315
# The raw report holds plaintext secrets.
316316
try:
317-
os.remove(report_path)
317+
os.remove(raw_report)
318318
except FileNotFoundError:
319319
pass
320320
except OSError as exc:

‎.github/security/test_secret_gate.py‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import ai_security_review as air
1818
import redaction
19+
import sanitize_findings
1920
import scan_head
2021

2122
# --------------------------------------------------------------------- helpers
@@ -593,6 +594,7 @@ def fake_run(args, **kw):
593594
scan_head.run_betterleaks(str(tmp_path), "cfg.toml", str(tmp_path / "r.json"))
594595
assert seen["dir"] != "/tmp/bl-noignore"
595596
assert seen["listing"] == []
597+
assert not os.path.exists(seen["dir"]), "left the ignore dir behind"
596598

597599

598600
@pytest.mark.parametrize("line, masked", [
@@ -623,3 +625,45 @@ def test_every_gate_workflow_is_valid_yaml():
623625
for f in sorted(workflows.glob("*.yml")):
624626
doc = yaml.safe_load(f.read_text())
625627
assert isinstance(doc, dict) and doc.get("jobs"), f.name
628+
629+
630+
# ------------------------------------------------- detections past the cap
631+
632+
633+
def test_detection_past_the_cap_is_masked_in_a_kept_window(monkeypatch):
634+
"""`redact` misses this value, so only the span mask keeps it from the model."""
635+
kept, dropped = "kkkkQ7zv", "hunter2-but-longer"
636+
lines = [f'first = "{kept}"', f'second = "{dropped}"']
637+
monkeypatch.setattr(air, "fetch_blob",
638+
lambda *a, **k: ("\n".join(lines).encode(), ""))
639+
640+
def det(fid, n, value):
641+
at = lines[n - 1].index(value)
642+
return {**_finding(fid, "generic-api-key", n, at + 1, at + len(value)),
643+
"location_kind": "line", "blob_sha": "cafe", "file": "a.py"}
644+
645+
first, second = det(1, 1, kept), det(2, 2, dropped)
646+
assert dropped in air.build_evidence("o/r", "tok", [first])[0][0]["window"]
647+
648+
evidence, errors = air.build_evidence("o/r", "tok", [first], [first, second])
649+
650+
assert errors == [] and len(evidence) == 1
651+
assert dropped not in evidence[0]["window"]
652+
assert "«DETECTION-2:" in evidence[0]["window"]
653+
654+
655+
# ------------------------------------------------- comment and fingerprint
656+
657+
658+
def test_no_green_tick_when_stage_1_did_not_finish_cleanly():
659+
review = {"findings": [], "false_positives": 0, "unanswered": [],
660+
"injection": False}
661+
assert "✅" in air.render_comment(review, "success", "")
662+
assert "✅" not in air.render_comment(review, "failure", "")
663+
664+
665+
def test_fingerprint_correlates_without_revealing_the_secret():
666+
a = sanitize_findings.fingerprint("s3cr3t-value")
667+
assert a == sanitize_findings.fingerprint("s3cr3t-value")
668+
assert a != sanitize_findings.fingerprint("s3cr3t-valuf")
669+
assert re.fullmatch(r"[0-9a-f]{12}", a), "carries more than a keyed digest"

‎.github/workflows/pr-security-scan.yml‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -190,17 +190,23 @@ jobs:
190190
fi
191191
192192
if [ -s /tmp/changed.z ]; then
193-
# Trailing `--` so a file named "--config=evil.py" is a path, not a flag.
194-
xargs -0 -a /tmp/changed.z \
193+
# xargs splits a long file list into several semgrep runs, so each run
194+
# writes its own report and they are merged below. An empty report
195+
# means that run did not complete (e.g. registry unreachable); it marks
196+
# the whole scan as not scanned rather than as zero findings. Trailing
197+
# `--` so a file named "--config=evil.py" is a path, not a flag.
198+
# shellcheck disable=SC2016 # the inner sh expands $out and $@
199+
xargs -0 -a /tmp/changed.z sh -c '
200+
out=$(mktemp /tmp/semgrep.part.XXXXXX)
195201
/tmp/sgvenv/bin/semgrep scan \
196202
--config=p/secrets --config=p/security-audit --config=p/owasp-top-ten \
197-
--json --output=/tmp/semgrep.raw.json \
198-
--metrics=off --quiet --timeout=120 -- || true
199-
# No report means the scan did not complete (e.g. registry unreachable);
200-
# mark it as not scanned rather than as zero findings.
201-
[ -f /tmp/semgrep.raw.json ] \
202-
|| echo '{"results":[],"__not_scanned":"scan did not complete"}' \
203-
> /tmp/semgrep.raw.json
203+
--json --output="$out" --metrics=off --quiet --timeout=120 -- "$@" || true
204+
[ -s "$out" ] || echo "{\"results\":[],\"__not_scanned\":\"scan did not complete\"}" > "$out"
205+
' sh || true
206+
jq -s '{results: (map(.results // []) | add)}
207+
+ (map(.__not_scanned // empty) | if length > 0 then {__not_scanned: .[0]} else {} end)' \
208+
/tmp/semgrep.part.* > /tmp/semgrep.raw.json \
209+
|| echo '{"results":[],"__not_scanned":"scan did not complete"}' > /tmp/semgrep.raw.json
204210
else
205211
echo '{"results":[]}' > /tmp/semgrep.raw.json
206212
fi

0 commit comments

Comments
 (0)