fix(bot): skip tests/ for secret patterns, basename forbidden match

[samson910022]

Follow-up precision tuning (option A, no real run yet). Secret-pattern scan skips tests/ before any I/O (fixtures are dummy keys by design); forbidden-file names still checked on every path via basename match (a committed tests/.env still fires as blocking). Live findings: 12 -> 2 (both nit in docs). 39 python tests green, review APPROVE.

[github-actions]

Verdict matrix

• Roles: gateway_safety, api_compat, docs_config
• FINAL_VERDICT: APPROVE

Sensitive paths

• github_bot/src/repo_scan.py

Deterministic pre-checks

• (none)

gateway_safety

VERDICT: APPROVE

Rationale: PR updates github_bot/src/repo_scan.py (lines 34-113) to skip PATTERN_SKIP_DIRS ("tests/") before pattern I/O in scan_worktree (reducing dummy-fixture noise) while fixing is_forbidden_tracked (lines 50-60) to basename-match FORBIDDEN_TRACKED + .env* rules so secrets in any dir (incl. tests/) still trigger; added tests/.env backstop test in tests/test_ai_bot.py:106 and test_tests_dir_skipped_for_patterns:371 with no auth, log, or lifecycle changes. All 8 checklist items satisfied; 39 Python tests green.

api_compat

API Compatibility Review

Summary

This PR modifies only the GitHub bot's repo-scan logic (github_bot/src/repo_scan.py) and its test suite (tests/test_ai_bot.py). No [provider] source (src/), converter, route handler, config builder, or wire-shape code is touched.

---

Checklist

#	Item	Status	Notes
1	Route error wire shapes preserved	✅ N/A	No route/handler changes
2	Converter thinking/signature blocks	✅ N/A	No converter changes
3	Session chaining / TTL / sweep	✅ N/A	Not touched
4	Reasoning mapping thresholds	✅ N/A	Not touched
5	Model alias rules	✅ N/A	Not touched
6	Collector finish/idle/flushHeaders	✅ N/A	Not touched
7	resolveMaxRetries / backoff	✅ N/A	Not touched
8	New endpoints documented	✅ N/A	No new endpoints

No cross-protocol regressions possible from this diff.

---

Findings

BLOCKING: (none)

SHOULD_FIX:

1. github_bot/src/repo_scan.py:53 – is_forbidden_tracked now matches basename only, broadening scope beyond intent.
   Previously is_forbidden_tracked("vendor/config.json") → False; now → True. This is the stated design ("a committed tests/.env still fires"), but it also means any vendored or nested config.json (e.g. node_modules/foo/config.json, examples/config.json) would fire as blocking if those paths ever reach the function. The .example guard doesn't help here. Consider restricting the broadening to PATTERN_SKIP_DIRS paths only, or adding an allowlist for known-safe nested occurrences, to avoid future false positives.

2. github_bot/src/repo_scan.py:118 – str.startswith with a tuple works in Python, but PATTERN_SKIP_DIRS is a 1-tuple; if a second entry is ever added without a trailing comma the tuple silently becomes a string and startswith still works but matches character-by-character. This is fragile. A brief inline comment or an assertion (assert isinstance(PATTERN_SKIP_DIRS, tuple)) would guard against that.

NITS:

1. github_bot/src/repo_scan.py:50 – The docstring says "Matched by basename so a committed secret trips in any directory (including PATTERN_SKIP_DIRS like tests/)." — the parenthetical is helpful but could note that node_modules/ paths are already excluded by the max_files / .gitignore filter upstream; without that context a reader might worry about the broadened match.

2. tests/test_ai_bot.py:385 – test_tests_dir_skipped_for_patterns iterates collect_findings() which does real I/O against the worktree. If the test suite is ever run in a minimal checkout (CI with sparse-checkout), this could produce zero findings and pass vacuously. A brief guard (self.assertTrue(len(list(collect_findings())) > 0, "sanity: scan produced findings")) would strengthen the assertion.

3. tests/test_ai_bot.py:359 – Comment update is accurate but the old comment text ("quoted fixture hit is nit") described a real downgrade path that's now gone; the new comment could mention that the nit-downgrade path in the loop below (lines 373-374) is still exercised for non-tests/ low-risk paths.

---

Test gaps

• No negative test for is_forbidden_tracked("vendor/config.json") — the broadened basename match is tested only for tests/.env. Adding a case for a nested non-test path would document the intended scope.
• No test that .env.production (no .example suffix) under a PATTERN_SKIP_DIRS path fires as forbidden.

---

VERDICT: APPROVE

The changes are scoped entirely to the bot's scan heuristics and do not touch any [provider] code, wire shapes, or API compatibility surfaces. The should-fix items are real but non-blocking — they affect scan precision, not runtime behavior.

docs_config

VERDICT: APPROVE

BLOCKING: none

SHOULD_FIX: none

NITS: none

Matrix-gap closure — files that must change: none.

• github_bot/src/repo_scan.py:37 PATTERN_SKIP_DIRS is internal-only, not env/file config — no six-way sync required across .env.example, config.json.example, Dockerfile, docker-compose.yml, index.ts, docs/configuration.md, AGENTS.md.
• github_bot/src/repo_scan.py:52-64 basename is_forbidden_tracked preserves safety invariant (committed tests/.env still blocking) without expanding tracked secret values; no doc link to git-ignored docs/host-deployments.md added.
• tests/test_ai_bot.py:109, 377-386 test-only change; no npm run lint / test:unit rename, no REQUEST_TIMEOUT_MS / MANAGE_BACKEND / OMIT_SYSTEM_PROMPT / DISABLE_TOOLS drift touched, no CHANGELOG.md user-facing entry required.

FINAL_VERDICT: APPROVE

Session: response_id=9c39fdfc-966e-961f-8ce9-23015d6a58d3,resp_req_vrtx_011CetUn5xeCU9MCGm4f746s,chatcmpl-30bce5fe-6bdb-4429-8455-be3e2c439e8f (chain via previous_response_id, 30min TTL). model=[model] provider=[provider].