Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions github_bot/src/repo_scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@
LOW_RISK_DIRS = ("tests/", "docs/")
LOW_RISK_SUFFIXES = (".yml", ".yaml", ".md")

# Test fixtures are dummy keys by design; pattern-scanning them only produces
# noise. Skipped for secret patterns (forbidden-file names are still checked
# above for every path, so a committed tests/.env still fires as blocking).
PATTERN_SKIP_DIRS = ("tests/",)


def is_low_risk_path(rel: str) -> bool:
"""True for fixture/doc/config paths where quoted hits are usually noise."""
Expand All @@ -45,10 +50,15 @@ def is_low_risk_path(rel: str) -> bool:


def is_forbidden_tracked(rel: str) -> bool:
"""True only for real secret files; never for `*.example` templates."""
if rel in FORBIDDEN_TRACKED:
"""True only for real secret files; never for `*.example` templates.

Matched by basename so a committed secret trips in any directory
(including PATTERN_SKIP_DIRS like `tests/`).
"""
base = rel.rsplit("/", 1)[-1]
if base in FORBIDDEN_TRACKED:
return True
if rel == ".env" or (rel.startswith(".env.") and not rel.endswith(".example")):
if base == ".env" or (base.startswith(".env.") and not base.endswith(".example")):
return True
return False
MATRIX_FILES = [
Expand Down Expand Up @@ -103,6 +113,11 @@ def scan_worktree(max_files: int = 400) -> list[dict[str, str]]:
# Tracked templates (`.env.example`, `config.json.example`) carry
# placeholder values by design; secret-pattern hits there are noise.
continue
if rel.startswith(PATTERN_SKIP_DIRS):
# Fixture keys under tests/ are dummy by design; skip before any
# I/O so they cost no file budget. Forbidden names already
# checked above, so a committed tests/.env still fires.
continue
path = REPO_ROOT / rel
if not path.is_file() or path.stat().st_size > 300_000:
continue
Expand Down
17 changes: 16 additions & 1 deletion tests/test_ai_bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ def test_scan_ignores_env_example(self):
self.assertTrue(is_forbidden_tracked(".env"))
self.assertTrue(is_forbidden_tracked(".env.local"))
self.assertTrue(is_forbidden_tracked("config.json"))
# Backstop holds inside skipped dirs: a committed tests/.env still fires.
self.assertTrue(is_forbidden_tracked("tests/.env"))

def test_scrub_covers_full_catalog(self):
from llm_client import INTERNAL_MODEL_NAMES
Expand Down Expand Up @@ -354,7 +356,8 @@ def test_low_risk_path_downgrade(self):

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.
# bare-code hit and unquoted yml env hit are gone; tests/ fixtures
# are skipped for patterns (see test_tests_dir_skipped_for_patterns).
from repo_scan import collect_findings, is_low_risk_path
by_loc = {}
for item in collect_findings():
Expand All @@ -371,6 +374,18 @@ def test_prior_dry_run_fps_resolved(self):
if slug in ("possible-api-key", "possible-password") and is_low_risk_path(path):
self.assertEqual(item["severity"], "nit", item["location"])

def test_tests_dir_skipped_for_patterns(self):
# Fixture keys under tests/ are dummy by design: no secret-pattern
# findings there (forbidden-file names are still checked).
from repo_scan import PATTERN_SKIP_DIRS, collect_findings
self.assertIn("tests/", PATTERN_SKIP_DIRS)
for item in collect_findings():
path = item["location"].split(":")[0]
slug = item["fingerprint"].split(":")[1]
if slug in ("possible-api-key", "possible-password",
"private-key-material"):
self.assertFalse(path.startswith("tests/"), 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")
Expand Down
Loading