Skip to content

Commit 466367a

Browse files
committed
fix(bot): scan precision - quoted secrets only, low-risk paths to nit
1 parent bd1bb42 commit 466367a

2 files changed

Lines changed: 64 additions & 3 deletions

File tree

‎github_bot/src/repo_scan.py‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,26 @@
1919

2020
SECRET_PATTERNS = [
2121
(r"-----BEGIN [A-Z ]*PRIVATE KEY-----", "private-key-material"),
22-
(r"(?i)(api[_-]?key|apikey)\s*[:=]\s*['\"]?[A-Za-z0-9._~+/-]{8,}", "possible-api-key"),
23-
(r"(?i)(password|passwd)\s*[:=]\s*['\"]?[^'\"\s]+", "possible-password"),
22+
# Quoted assignments only. Bare code (`self.api_key = lookup(...)`) and
23+
# unquoted env lines (`API_KEY= SOME_VAR`) are noise; pasted secrets are
24+
# almost always quoted. Committed `.env` files are caught separately by
25+
# is_forbidden_tracked, so recall on real leaks is preserved.
26+
(r"(?i)(api[_-]?key|apikey)\s*[:=]\s*['\"][^'\"]{3,}['\"]", "possible-api-key"),
27+
(r"(?i)(password|passwd)\s*[:=]\s*['\"][^'\"]{3,}['\"]", "possible-password"),
2428
]
2529
SEVERITY_RANK = {"blocking": 0, "should-fix": 1, "nit": 2}
2630

31+
# Secret-pattern hits under these paths are overwhelmingly fixtures/docs, not
32+
# leaks. Downgraded to nit so real findings sort first; fingerprints stay
33+
# stable so reruns still dedupe.
34+
LOW_RISK_DIRS = ("tests/", "docs/")
35+
LOW_RISK_SUFFIXES = (".yml", ".yaml", ".md")
36+
37+
38+
def is_low_risk_path(rel: str) -> bool:
39+
"""True for fixture/doc/config paths where quoted hits are usually noise."""
40+
return rel.startswith(LOW_RISK_DIRS) or rel.endswith(LOW_RISK_SUFFIXES)
41+
2742
# Exact tracked paths that must never be committed (values, not templates).
2843
# Note: `.env.example` is an intentionally tracked template and must NOT match.
2944
FORBIDDEN_TRACKED = (".env", "config.json", "opencode.json")
@@ -105,10 +120,12 @@ def scan_worktree(max_files: int = 400) -> list[dict[str, str]]:
105120
# One finding per (rule, file): fingerprints differ by slug so coexisting
106121
# rules on the same file each get their own stable id.
107122
reported_rules: set[str] = set()
123+
low_risk = is_low_risk_path(rel)
108124
for i, line in enumerate(text.splitlines(), 1):
109125
for pattern, rule in SECRET_PATTERNS:
110126
if rule not in reported_rules and re.search(pattern, line):
111-
findings.append(_finding(rule, rel, "blocking", "[REDACTED line]", i))
127+
severity = "nit" if low_risk else "blocking"
128+
findings.append(_finding(rule, rel, severity, "[REDACTED line]", i))
112129
reported_rules.add(rule)
113130
findings.sort(key=lambda d: (SEVERITY_RANK.get(d["severity"], 9), d["fingerprint"]))
114131
return findings

‎tests/test_ai_bot.py‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,50 @@ def test_fingerprint_stable(self):
327327
self.assertEqual(fingerprint("slug", "a/b.ts"), fingerprint("slug", "a/b.ts"))
328328
self.assertTrue(fingerprint("slug", "a/b.ts").startswith("opencode2api-scan:"))
329329

330+
def test_secret_patterns_require_quotes(self):
331+
import re
332+
from repo_scan import SECRET_PATTERNS
333+
by_rule = {rule: pattern for pattern, rule in SECRET_PATTERNS}
334+
# Real pasted secrets (quoted) still match.
335+
self.assertTrue(re.search(by_rule["possible-api-key"], 'api_key = "sk-live-abc123"'))
336+
self.assertTrue(re.search(by_rule["possible-password"], "password: 'hunter2-hunter2'"))
337+
self.assertTrue(re.search(by_rule["private-key-material"],
338+
"-----BEGIN PRIVATE KEY-----"))
339+
# Bare code and unquoted env lines are noise, not findings.
340+
self.assertFalse(re.search(
341+
by_rule["possible-api-key"], 'self.api_key = default_pdata.get("apikey", "")'))
342+
self.assertFalse(re.search(
343+
by_rule["possible-api-key"], "API_KEY= OPENCODE_PROXY_MANAGE_BACKEND=true"))
344+
self.assertFalse(re.search(
345+
by_rule["possible-password"], "password = get_password()"))
346+
347+
def test_low_risk_path_downgrade(self):
348+
from repo_scan import is_low_risk_path
349+
self.assertTrue(is_low_risk_path("tests/auth-multikey.test.js"))
350+
self.assertTrue(is_low_risk_path("docs/docker.md"))
351+
self.assertTrue(is_low_risk_path(".github/workflows/ai-review.yml"))
352+
self.assertFalse(is_low_risk_path("src/proxy.ts"))
353+
self.assertFalse(is_low_risk_path("github_bot/src/llm_client.py"))
354+
355+
def test_prior_dry_run_fps_resolved(self):
356+
# The three dry-run false positives from the first live scan:
357+
# bare-code hit, unquoted yml env hit gone; quoted fixture hit is nit.
358+
from repo_scan import collect_findings, is_low_risk_path
359+
by_loc = {}
360+
for item in collect_findings():
361+
by_loc.setdefault(item["location"].split(":")[0], []).append(item)
362+
llm_hits = [i for i in by_loc.get("github_bot/src/llm_client.py", [])
363+
if "possible-api-key" in i["fingerprint"]]
364+
self.assertEqual(llm_hits, [])
365+
yml_hits = [i for i in by_loc.get(".github/workflows/ai-review.yml", [])
366+
if "possible-api-key" in i["fingerprint"]]
367+
self.assertEqual(yml_hits, [])
368+
for path, items in by_loc.items():
369+
for item in items:
370+
slug = item["fingerprint"].split(":")[1]
371+
if slug in ("possible-api-key", "possible-password") and is_low_risk_path(path):
372+
self.assertEqual(item["severity"], "nit", item["location"])
373+
330374
def test_comment_modes(self):
331375
self.assertEqual(github_runner._resolve_comment_mode("/review please"), "review")
332376
self.assertEqual(github_runner._resolve_comment_mode("/fix this"), "fix-plan")

0 commit comments

Comments
 (0)