From 466367ae9bcd17c39ae9a6da9e13eae08413ba47 Mon Sep 17 00:00:00 2001 From: samson910022 Date: Thu, 10 Sep 2026 04:07:19 +0800 Subject: [PATCH] fix(bot): scan precision - quoted secrets only, low-risk paths to nit --- github_bot/src/repo_scan.py | 23 ++++++++++++++++--- tests/test_ai_bot.py | 44 +++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/github_bot/src/repo_scan.py b/github_bot/src/repo_scan.py index caae11a..a7a68c4 100644 --- a/github_bot/src/repo_scan.py +++ b/github_bot/src/repo_scan.py @@ -19,11 +19,26 @@ SECRET_PATTERNS = [ (r"-----BEGIN [A-Z ]*PRIVATE KEY-----", "private-key-material"), - (r"(?i)(api[_-]?key|apikey)\s*[:=]\s*['\"]?[A-Za-z0-9._~+/-]{8,}", "possible-api-key"), - (r"(?i)(password|passwd)\s*[:=]\s*['\"]?[^'\"\s]+", "possible-password"), + # Quoted assignments only. Bare code (`self.api_key = lookup(...)`) and + # unquoted env lines (`API_KEY= SOME_VAR`) are noise; pasted secrets are + # almost always quoted. Committed `.env` files are caught separately by + # is_forbidden_tracked, so recall on real leaks is preserved. + (r"(?i)(api[_-]?key|apikey)\s*[:=]\s*['\"][^'\"]{3,}['\"]", "possible-api-key"), + (r"(?i)(password|passwd)\s*[:=]\s*['\"][^'\"]{3,}['\"]", "possible-password"), ] SEVERITY_RANK = {"blocking": 0, "should-fix": 1, "nit": 2} +# Secret-pattern hits under these paths are overwhelmingly fixtures/docs, not +# leaks. Downgraded to nit so real findings sort first; fingerprints stay +# stable so reruns still dedupe. +LOW_RISK_DIRS = ("tests/", "docs/") +LOW_RISK_SUFFIXES = (".yml", ".yaml", ".md") + + +def is_low_risk_path(rel: str) -> bool: + """True for fixture/doc/config paths where quoted hits are usually noise.""" + return rel.startswith(LOW_RISK_DIRS) or rel.endswith(LOW_RISK_SUFFIXES) + # Exact tracked paths that must never be committed (values, not templates). # Note: `.env.example` is an intentionally tracked template and must NOT match. FORBIDDEN_TRACKED = (".env", "config.json", "opencode.json") @@ -105,10 +120,12 @@ def scan_worktree(max_files: int = 400) -> list[dict[str, str]]: # One finding per (rule, file): fingerprints differ by slug so coexisting # rules on the same file each get their own stable id. reported_rules: set[str] = set() + low_risk = is_low_risk_path(rel) for i, line in enumerate(text.splitlines(), 1): for pattern, rule in SECRET_PATTERNS: if rule not in reported_rules and re.search(pattern, line): - findings.append(_finding(rule, rel, "blocking", "[REDACTED line]", i)) + severity = "nit" if low_risk else "blocking" + findings.append(_finding(rule, rel, severity, "[REDACTED line]", i)) reported_rules.add(rule) findings.sort(key=lambda d: (SEVERITY_RANK.get(d["severity"], 9), d["fingerprint"])) return findings diff --git a/tests/test_ai_bot.py b/tests/test_ai_bot.py index 6c3a8bf..e7b614e 100644 --- a/tests/test_ai_bot.py +++ b/tests/test_ai_bot.py @@ -327,6 +327,50 @@ def test_fingerprint_stable(self): self.assertEqual(fingerprint("slug", "a/b.ts"), fingerprint("slug", "a/b.ts")) self.assertTrue(fingerprint("slug", "a/b.ts").startswith("opencode2api-scan:")) + def test_secret_patterns_require_quotes(self): + import re + from repo_scan import SECRET_PATTERNS + by_rule = {rule: pattern for pattern, rule in SECRET_PATTERNS} + # Real pasted secrets (quoted) still match. + self.assertTrue(re.search(by_rule["possible-api-key"], 'api_key = "sk-live-abc123"')) + self.assertTrue(re.search(by_rule["possible-password"], "password: 'hunter2-hunter2'")) + self.assertTrue(re.search(by_rule["private-key-material"], + "-----BEGIN PRIVATE KEY-----")) + # Bare code and unquoted env lines are noise, not findings. + self.assertFalse(re.search( + by_rule["possible-api-key"], 'self.api_key = default_pdata.get("apikey", "")')) + self.assertFalse(re.search( + by_rule["possible-api-key"], "API_KEY= OPENCODE_PROXY_MANAGE_BACKEND=true")) + self.assertFalse(re.search( + by_rule["possible-password"], "password = get_password()")) + + def test_low_risk_path_downgrade(self): + from repo_scan import is_low_risk_path + self.assertTrue(is_low_risk_path("tests/auth-multikey.test.js")) + self.assertTrue(is_low_risk_path("docs/docker.md")) + self.assertTrue(is_low_risk_path(".github/workflows/ai-review.yml")) + self.assertFalse(is_low_risk_path("src/proxy.ts")) + self.assertFalse(is_low_risk_path("github_bot/src/llm_client.py")) + + def test_prior_dry_run_fps_resolved(self): + # The three dry-run false positives from the first live scan: + # bare-code hit, unquoted yml env hit gone; quoted fixture hit is nit. + from repo_scan import collect_findings, is_low_risk_path + by_loc = {} + for item in collect_findings(): + by_loc.setdefault(item["location"].split(":")[0], []).append(item) + llm_hits = [i for i in by_loc.get("github_bot/src/llm_client.py", []) + if "possible-api-key" in i["fingerprint"]] + self.assertEqual(llm_hits, []) + yml_hits = [i for i in by_loc.get(".github/workflows/ai-review.yml", []) + if "possible-api-key" in i["fingerprint"]] + self.assertEqual(yml_hits, []) + for path, items in by_loc.items(): + for item in items: + slug = item["fingerprint"].split(":")[1] + if slug in ("possible-api-key", "possible-password") and is_low_risk_path(path): + self.assertEqual(item["severity"], "nit", item["location"]) + def test_comment_modes(self): self.assertEqual(github_runner._resolve_comment_mode("/review please"), "review") self.assertEqual(github_runner._resolve_comment_mode("/fix this"), "fix-plan")