gitutil.py 31 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403404405406407408409410411412413414415416417418419420421422423424425426427428429430431432433434435436437438439440441442443444445446447448449450451452453454455456457458459460461462463464465466467468469470471472473474475476477478479480481482483484485486487488489490491492493494495496497498499500501502503504505506507508509510511512513514515516517518519520521522523524525526527528529530531532533534535536537538539540541542543544545546547548549550551552553554555556557558559560561562563564565566567568569570571572573574575576577578579580581582583584585586587588589590591592593594595596597598599600601602603604605606607608609610611612613614615616617618619620621622623624625626627628629630631632633634635636637638639640641642643644645646647648649650651652653654655656657658659660661662663664665666667668669670671672673674675676677678679680681682683684685686687688689690691692693694695696697698699700701702703704705706707708709710711712713714715716717718719720721722723724725726727728729730731732733734735736737738739740741742743744745746747748
  1. """
  2. Leaf git/subprocess helpers and diff parsing for the security-guidance plugin.
  3. Everything here is a thin wrapper over ``git``/``subprocess`` plus pure
  4. diff-text parsing and source-file classification. None of these functions
  5. reference any name that the test suite monkeypatches on
  6. ``security_reminder_hook`` and then calls *through* another function in this
  7. module — that property is what makes them safe to live in their own module
  8. while still being re-exported (so tests that patch ``hook._git_toplevel`` and
  9. then call a handler in ``security_reminder_hook`` continue to see the patched
  10. binding).
  11. Functions that DO compose patched leaves (``compute_v2_review_set``,
  12. ``_list_untracked``, ``_append_reviewed_shas``) deliberately remain in
  13. ``security_reminder_hook.py`` for that reason.
  14. """
  15. import contextlib
  16. import os
  17. import re
  18. import subprocess
  19. from _base import debug_log
  20. GIT_CMD = [
  21. "git",
  22. "-c", "core.fsmonitor=false",
  23. "-c", "core.hooksPath=/dev/null",
  24. ]
  25. def _git_rev_parse_head(cwd):
  26. """Return the current HEAD SHA, or None if not a git repo / no commits."""
  27. try:
  28. result = subprocess.run(
  29. [*GIT_CMD, "rev-parse", "HEAD"],
  30. cwd=cwd, capture_output=True, text=True, timeout=5
  31. )
  32. if result.returncode == 0 and result.stdout.strip():
  33. return result.stdout.strip()
  34. return None
  35. except (subprocess.TimeoutExpired, FileNotFoundError, OSError):
  36. return None
  37. def _find_git_index(cwd):
  38. """
  39. Find the real index file for a git repo. Handles worktrees where .git
  40. is a file pointing to the main repo's gitdir.
  41. Returns the absolute path to the index file, or None.
  42. """
  43. try:
  44. result = subprocess.run(
  45. [*GIT_CMD, "rev-parse", "--git-dir"],
  46. cwd=cwd, capture_output=True, text=True, timeout=5
  47. )
  48. if result.returncode != 0:
  49. return None
  50. git_dir = result.stdout.strip()
  51. if not os.path.isabs(git_dir):
  52. git_dir = os.path.join(cwd, git_dir)
  53. index_path = os.path.join(git_dir, "index")
  54. return index_path if os.path.isfile(index_path) else None
  55. except (subprocess.TimeoutExpired, FileNotFoundError, OSError):
  56. return None
  57. def _diff_pathspec(cwd, paths):
  58. """Convert absolute touched-paths to repo-relative pathspec args for
  59. git diff. Paths outside cwd (e.g. ~/.claude/…) are dropped. Returns the
  60. list to splice after `--`, or [] for an unrestricted diff. realpath both
  61. sides so the macOS /var ↔ /private/var symlink doesn't make in-repo
  62. paths look external."""
  63. if not paths:
  64. return []
  65. cwd_abs = os.path.realpath(cwd)
  66. rel = []
  67. for p in paths:
  68. try:
  69. r = os.path.relpath(os.path.realpath(p), cwd_abs)
  70. except ValueError:
  71. continue
  72. if r.startswith(".."):
  73. continue
  74. rel.append(r)
  75. return ["--"] + rel if rel else []
  76. @contextlib.contextmanager
  77. def _temp_index(cwd, untracked_paths=None):
  78. """Yield an env dict pointing GIT_INDEX_FILE at a throwaway copy of the
  79. repo's index with `git add --intent-to-add` applied, so untracked files
  80. show up in subsequent `git diff` calls without touching the user's real
  81. index. Yields None if no index can be found (bare repo / not a repo); the
  82. caller should fall back to a plain diff. Always cleans up the temp file.
  83. Perf: when `untracked_paths` is given, only those paths are added (O(n)
  84. in untracked count). The default `add -N .` stats every file in the
  85. worktree — slow in large repos vs fast targeted scan. v2 callers
  86. already know the untracked set from `git status --porcelain`, so they
  87. pass it; v1 keeps the whole-tree scan since it has no prior list."""
  88. import shutil
  89. import tempfile
  90. real_index = _find_git_index(cwd)
  91. if not real_index:
  92. yield None
  93. return
  94. tmp_fd, tmp_index = tempfile.mkstemp(prefix="security_hook_idx_")
  95. os.close(tmp_fd)
  96. try:
  97. shutil.copy2(real_index, tmp_index)
  98. env = {**os.environ, "GIT_INDEX_FILE": tmp_index}
  99. if untracked_paths is None:
  100. add_args = ["."]
  101. elif untracked_paths:
  102. # `git add -N -- a b nonexistent` is atomic — one missing path
  103. # makes it exit 128 and add NOTHING, so a file removed between
  104. # `git status` and here would silently drop ALL untracked files
  105. # from the diff. --ignore-missing only works with --dry-run, so
  106. # filter to surviving paths (lexists so dangling symlinks count).
  107. surviving = [p for p in untracked_paths
  108. if os.path.lexists(os.path.join(cwd, p))]
  109. add_args = ["--"] + surviving if surviving else None
  110. else:
  111. add_args = None
  112. if add_args:
  113. subprocess.run(
  114. [*GIT_CMD, "add", "--intent-to-add"] + add_args,
  115. cwd=cwd, capture_output=True, text=True, timeout=10,
  116. env=env,
  117. )
  118. yield env
  119. finally:
  120. try:
  121. os.unlink(tmp_index)
  122. except OSError:
  123. pass
  124. def _git_toplevel(cwd):
  125. """Absolute repo root for `cwd`, or None if not in a work tree."""
  126. try:
  127. r = subprocess.run(
  128. [*GIT_CMD, "rev-parse", "--show-toplevel"],
  129. cwd=cwd, capture_output=True, text=True, timeout=5,
  130. )
  131. return r.stdout.strip() if r.returncode == 0 and r.stdout.strip() else None
  132. except (subprocess.TimeoutExpired, FileNotFoundError, OSError):
  133. return None
  134. def _git_dir(repo_root):
  135. """Absolute shared `.git` directory for repo_root.
  136. Uses `rev-parse --git-common-dir` so linked worktrees resolve to the
  137. SHARED gitdir, not the per-worktree `.git/worktrees/<name>/`. That way
  138. push-sweep's reviewed-shas record (and the bash-hook-once sentinel)
  139. is per-clone — a commit reviewed in one worktree counts as reviewed
  140. if a different worktree later pushes it. Returns None on failure so
  141. callers can degrade (push-sweep state is best-effort).
  142. """
  143. try:
  144. r = subprocess.run(
  145. [*GIT_CMD, "rev-parse", "--git-common-dir"],
  146. cwd=repo_root, capture_output=True, text=True, timeout=5,
  147. )
  148. if r.returncode != 0:
  149. return None
  150. d = r.stdout.strip()
  151. return d if os.path.isabs(d) else os.path.join(repo_root, d)
  152. except (subprocess.TimeoutExpired, FileNotFoundError, OSError):
  153. return None
  154. def _git_rev_list_range(repo_root, base, head="HEAD"):
  155. """Shas in `base..head`, oldest→newest. Empty list on error."""
  156. try:
  157. r = subprocess.run(
  158. [*GIT_CMD, "rev-list", "--reverse", f"{base}..{head}"],
  159. cwd=repo_root, capture_output=True, text=True, timeout=10,
  160. )
  161. if r.returncode != 0:
  162. return []
  163. return [s for s in r.stdout.strip().split("\n") if s]
  164. except (subprocess.TimeoutExpired, FileNotFoundError, OSError):
  165. return []
  166. def _git_diff_range(repo_root, base, head="HEAD"):
  167. """`git diff -p base head` as text on success, None on error.
  168. Distinguishing failure from success-with-empty-diff matters: the push-sweep
  169. caller marks the tail reviewed when the diff is empty (nothing to review),
  170. but on failure (timeout, non-zero exit, missing git) it must NOT mark
  171. them reviewed — otherwise unreviewed commits get permanently silenced.
  172. """
  173. try:
  174. r = subprocess.run(
  175. [*GIT_CMD, "diff", "-p", "--no-color", "--no-ext-diff", base, head],
  176. cwd=repo_root, capture_output=True, timeout=30,
  177. )
  178. if r.returncode != 0:
  179. return None
  180. return r.stdout.decode("utf-8", errors="replace")
  181. except (subprocess.TimeoutExpired, FileNotFoundError, OSError):
  182. return None
  183. def _detect_main_branch(repo_root):
  184. for ref in ("origin/HEAD", "origin/main", "origin/master", "main", "master"):
  185. try:
  186. r = subprocess.run(
  187. [*GIT_CMD, "rev-parse", "--verify", "-q", ref],
  188. cwd=repo_root, capture_output=True, text=True, timeout=5,
  189. )
  190. if r.returncode == 0 and r.stdout.strip():
  191. return ref
  192. except (subprocess.TimeoutExpired, FileNotFoundError, OSError):
  193. pass
  194. return None
  195. def _git_reflog_recent_commits(repo_root, max_age_s=120, max_n=5):
  196. """Return (fresh_commit_shas, stale_count) from the HEAD reflog.
  197. Scans the last `max_n` reflog entries and returns the SHAs whose action is
  198. `commit*` AND whose commit timestamp is within `max_age_s` of now,
  199. newest-first. `stale_count` is the number of commit-action entries that
  200. were too old (so the caller can distinguish "no commit happened" from
  201. "commit happened earlier than the window").
  202. Used by commit-review when stdout-based `[branch sha]` detection fails
  203. (output piped/redirected/-q, or a chained command after `git commit`
  204. pushed the success line off — `git commit && git push` makes HEAD@{0}
  205. `update by push`, not `commit:`). The HEAD@{0}-only check
  206. keeps the not-yet-visible-HEAD skip rare; analysis showed the
  207. residual is dominated by these chained-command and noop-guard cases.
  208. Safety vs. blindly reading HEAD:
  209. - cross-repo (`cd ../other && git commit`): repo_root's own reflog has
  210. no fresh commit, so this returns ([], 0).
  211. - commit actually failed (pre-commit reject, nothing-staged): reflog's
  212. recent entries are the prior checkout/commit/reset → ([], 0) or only
  213. stale entries.
  214. - HEAD raced ahead (a second commit landed before this async hook ran):
  215. both commits appear in the scan and both get reviewed — correct.
  216. - prior Bash call's commit within the window: would be returned here,
  217. but the call site deduplicates against `.git/sg-reviewed-shas` so a
  218. SHA is reviewed at most once. This is also the non-overlap invariant
  219. with push-sweep.
  220. """
  221. if not repo_root:
  222. return [], 0
  223. try:
  224. # %gs (the reflog subject) is `commit: <commit-msg first line>` and can
  225. # contain `|`; put it LAST so split("|", 2) leaves it intact. %H is
  226. # hex and %ct is integer, so the first two fields are delimiter-safe.
  227. #
  228. # Bytes + decode utf-8/replace: %gs embeds commit-message subjects
  229. # which git stores as raw bytes — commits can be authored in
  230. # latin-1 / cp1252 / shift-jis etc., and text=True would raise
  231. # UnicodeDecodeError in the subprocess reader thread on Windows
  232. # cp1252 (subprocess.run returns r.stdout=None, then
  233. # r.stdout.splitlines() AttributeErrors). Mirrors the existing
  234. # migration at security_reminder_hook.py:540 — same pattern was
  235. # missed here. See anthropics/claude-plugins-official#2056.
  236. r = subprocess.run(
  237. [*GIT_CMD, "log", "-g", "-n", str(max_n),
  238. "--format=%H|%ct|%gs", "HEAD"],
  239. cwd=repo_root, capture_output=True, timeout=5,
  240. )
  241. except (subprocess.TimeoutExpired, FileNotFoundError, OSError, ValueError):
  242. return [], 0
  243. if r.returncode != 0:
  244. return [], 0
  245. stdout = (r.stdout or b"").decode("utf-8", errors="replace")
  246. import time as _time
  247. now = int(_time.time())
  248. fresh, stale = [], 0
  249. for idx, line in enumerate(stdout.splitlines()):
  250. parts = line.split("|", 2)
  251. if len(parts) != 3:
  252. continue
  253. sha, ct, subject = parts
  254. # `commit: msg`, `commit (amend): msg`, `commit (initial): msg`,
  255. # `commit (merge): msg` — all create a reviewable commit object.
  256. if not subject.startswith("commit"):
  257. continue
  258. try:
  259. age = now - int(ct)
  260. except ValueError:
  261. continue
  262. # HEAD@{0} (idx==0) is exempt from the age gate. The gate exists to
  263. # bound the WIDENED HEAD@{1..max_n-1} scan from picking up commits
  264. # made by *prior* Bash calls; HEAD@{0} is by definition the most
  265. # recent reflog entry and was previously accepted unconditionally
  266. # (_git_reflog_head_if_just_committed previously had no age check).
  267. # Applying max_age_s to idx==0 made the not-yet-visible-HEAD skip
  268. # noticeably more frequent on chained
  269. # `git commit && <slow command>` where %ct is >120s old by the
  270. # time the async PostToolUse hook fires.
  271. if idx == 0 or age <= max_age_s:
  272. fresh.append(sha)
  273. else:
  274. stale += 1
  275. return fresh, stale
  276. def _git_name_only(cwd, base, include_untracked=False):
  277. """Return the set of repo-root-relative paths that differ from `base`,
  278. or None if git failed (unresolvable ref, not a repo, timeout). Callers
  279. must distinguish None (error → don't trust as a filter) from set()
  280. (genuinely nothing changed). `-c core.quotePath=false -z` keeps non-ASCII
  281. and space-containing paths intact."""
  282. # Decode stdout/stderr as UTF-8 with errors="replace" instead of using
  283. # text=True. core.quotePath=false makes git emit raw UTF-8 for non-ASCII
  284. # paths, and text=True on Windows decodes via cp1252 strict — a non-ASCII
  285. # changed path would crash the subprocess reader thread, leave
  286. # result.stdout=None, and propagate AttributeError out of the helper.
  287. # Same fix shape as diffstate._list_untracked. See #2056.
  288. def _run(env):
  289. result = subprocess.run(
  290. [*GIT_CMD, "-c", "core.quotePath=false", "diff", "--name-only", "-z", base],
  291. cwd=cwd, capture_output=True, timeout=30,
  292. env=env,
  293. )
  294. if result.returncode != 0:
  295. stderr_str = (result.stderr or b"").decode("utf-8", errors="replace")
  296. debug_log(f"_git_name_only({base!r}) rc={result.returncode}: {stderr_str[:200]}")
  297. return None
  298. stdout = (result.stdout or b"").decode("utf-8", errors="replace")
  299. return {p for p in stdout.split("\0") if p}
  300. try:
  301. if not include_untracked:
  302. return _run(None)
  303. with _temp_index(cwd) as env:
  304. return _run(env)
  305. except (subprocess.TimeoutExpired, FileNotFoundError, OSError, ValueError) as e:
  306. debug_log(f"_git_name_only({base!r}) error: {e}")
  307. return None
  308. def _git_status_porcelain(cwd):
  309. """One `git status --porcelain=v1 -z` → (tracked_dirty, untracked) sets of
  310. repo-root-relative paths, or (None, None) on error. Replaces the
  311. `_temp_index + git diff HEAD --name-only` pair for the v2 dirty_now
  312. computation: faster in large repos, and yields the
  313. untracked set separately so the later get_git_diff can do a targeted
  314. `add -N -- <files>` instead of a whole-tree `add -N .`.
  315. -uall: list individual files inside untracked directories (default
  316. collapses to `dir/`). Required so the untracked set subtracts cleanly
  317. against the UPS-time `_list_untracked` snapshot, which uses ls-files and
  318. therefore always lists individual files."""
  319. # Lenient decode: same UTF-8 + errors="replace" pattern as the
  320. # sibling helpers — a non-ASCII path in the worktree would otherwise
  321. # crash the cp1252 reader thread on Windows. See #2056.
  322. try:
  323. r = subprocess.run(
  324. [*GIT_CMD, "-c", "core.quotePath=false", "status",
  325. "--porcelain=v1", "-uall", "-z"],
  326. cwd=cwd, capture_output=True, timeout=30,
  327. )
  328. if r.returncode != 0:
  329. stderr_str = (r.stderr or b"").decode("utf-8", errors="replace")
  330. debug_log(f"_git_status_porcelain rc={r.returncode}: {stderr_str[:200]}")
  331. return None, None
  332. tracked, untracked = set(), set()
  333. stdout = (r.stdout or b"").decode("utf-8", errors="replace")
  334. entries = stdout.split("\0")
  335. i = 0
  336. while i < len(entries):
  337. e = entries[i]
  338. if not e:
  339. i += 1
  340. continue
  341. xy, path = e[:2], e[3:]
  342. if xy == "??":
  343. untracked.add(path)
  344. else:
  345. tracked.add(path)
  346. # Rename/copy entries are XY old\0new\0 — second NUL field is
  347. # the origin path; consume it so it isn't misparsed as a new
  348. # 2-char-status entry.
  349. if "R" in xy or "C" in xy:
  350. i += 1
  351. i += 1
  352. return tracked, untracked
  353. except (subprocess.TimeoutExpired, FileNotFoundError, OSError, ValueError) as e:
  354. # ValueError guards against any future strict-decode regression
  355. # so the helper degrades to (None, None) instead of crashing.
  356. debug_log(f"_git_status_porcelain error: {e}")
  357. return None, None
  358. def _is_ancestor(cwd, maybe_ancestor, descendant):
  359. """True if `maybe_ancestor` is reachable from `descendant` (i.e. HEAD
  360. moved forward via commit/merge, not sideways via checkout)."""
  361. try:
  362. result = subprocess.run(
  363. [*GIT_CMD, "merge-base", "--is-ancestor", maybe_ancestor, descendant],
  364. cwd=cwd, capture_output=True, text=True, timeout=5,
  365. )
  366. return result.returncode == 0
  367. except (subprocess.TimeoutExpired, FileNotFoundError, OSError):
  368. return False
  369. def get_git_diff(cwd, baseline_sha, full_context=False, paths=None, untracked_paths=None):
  370. """
  371. Get the git diff between the baseline SHA and the current working tree,
  372. including untracked (new) files.
  373. Uses a temporary copy of the git index (GIT_INDEX_FILE) so the user's
  374. real index is never modified. The temp index gets intent-to-add entries
  375. for untracked files, making them visible in the diff output. Cleanup
  376. is just deleting the temp file in a finally block.
  377. If `paths` is given, the diff is restricted to those paths (relative to
  378. cwd; absolute paths are converted, paths outside cwd are dropped).
  379. `untracked_paths` (repo-root-relative) is forwarded to _temp_index so it
  380. can add only those files instead of scanning the whole worktree.
  381. """
  382. pathspec = _diff_pathspec(cwd, paths)
  383. if paths and not pathspec:
  384. # Caller restricted to specific paths but none are inside this repo
  385. # (e.g. only ~/.claude/... edits). Returning "" flows to skip(6); an
  386. # empty pathspec would mean an UNRESTRICTED diff — the bug this whole
  387. # change exists to fix.
  388. return ""
  389. cmd = [*GIT_CMD, "diff", "--no-color", "--no-ext-diff", baseline_sha] + (["--unified=99999"] if full_context else []) + pathspec
  390. try:
  391. with _temp_index(cwd, untracked_paths) as env:
  392. # env is None when no index could be found (bare repo / not a
  393. # repo) — diff still runs, just without untracked-file support.
  394. result = subprocess.run(cmd, cwd=cwd, capture_output=True, timeout=30, env=env)
  395. if result.returncode != 0:
  396. debug_log(f"git diff failed: {result.stderr[:200].decode('utf-8', errors='replace')}")
  397. return None
  398. # Decode with errors='replace' so binary diffs don't crash
  399. return result.stdout.decode("utf-8", errors="replace")
  400. except (subprocess.TimeoutExpired, FileNotFoundError, OSError) as e:
  401. debug_log(f"git diff error: {e}")
  402. return None
  403. # Source file extensions worth reviewing for security
  404. SOURCE_CODE_EXTENSIONS = {
  405. '.py', '.js', '.ts', '.jsx', '.tsx', '.go', '.java', '.rb', '.php',
  406. '.rs', '.c', '.cpp', '.h', '.hpp', '.cs', '.swift', '.kt', '.scala',
  407. '.html', '.htm', '.ejs', '.yaml', '.yml', '.properties',
  408. '.mjs', '.cjs', '.mts', '.cts', '.vue', '.svelte',
  409. '.sh', '.bash', '.zsh', '.fish', '.ksh', '.ps1', '.sql',
  410. '.gradle', '.groovy',
  411. '.tf', '.hcl', '.tfvars',
  412. '.json', '.toml', '.ipynb',
  413. }
  414. # Reviewable files identified by basename rather than extension (lowercased).
  415. # These are by-convention extensionless but contain executable recipes/DSL
  416. # with shell/exec surface (Make recipes, Jenkinsfile Groovy, Rakefile Ruby).
  417. SOURCE_CODE_BASENAMES = {
  418. 'dockerfile', 'makefile', 'gnumakefile', 'jenkinsfile', 'vagrantfile',
  419. 'rakefile', 'gemfile', 'procfile', 'brewfile', 'justfile',
  420. }
  421. # Extensionless basenames that are NOT source — plain-text metadata. Anything
  422. # extensionless not in this set is treated as source (likely a shebang script
  423. # under bin/ or scripts/). Analysis of skipped reviews found
  424. # extensionless executables (bin/deploy, scripts/run-canary) were the largest
  425. # remaining false-negative class — they carry shell-injection surface but
  426. # `splitext` gives '' so they were filtered out. _cap_files_for_prompt bounds
  427. # the byte cost downstream, and the reviewer ignores prose, so opting
  428. # extensionless IN with this small deny-list is the better default than
  429. # opting OUT.
  430. NON_SOURCE_EXTENSIONLESS_BASENAMES = {
  431. 'license', 'licence', 'copying', 'notice', 'patents', 'authors',
  432. 'contributors', 'maintainers', 'changelog', 'changes', 'news',
  433. 'readme', 'todo', 'install', 'version', 'codeowners',
  434. 'owners', 'copyright',
  435. }
  436. # Directory components and file suffixes that are never worth reviewing even
  437. # when the extension is in SOURCE_CODE_EXTENSIONS — vendored deps, build
  438. # output, generated code, minified bundles, lockfiles, protobuf stubs.
  439. # Matched as path *components* (so `node_modules/` matches anywhere in the
  440. # path, not just as a prefix) and as case-sensitive suffixes (the ecosystems
  441. # that emit `.min.js` / `_pb2.py` / `.pb.go` are case-consistent).
  442. SKIP_PATH_PATTERNS = (
  443. 'node_modules/', 'dist/', 'build/', '.next/', 'vendor/',
  444. '__generated__/', '__pycache__/', '.venv/', 'target/',
  445. )
  446. SKIP_FILE_SUFFIXES = (
  447. '.min.js', '.min.css', '.d.ts', '.d.mts', '.d.cts',
  448. '.lock', '_pb2.py', '.pb.go',
  449. )
  450. # Path tokens that bump a file's review priority when a commit exceeds
  451. # MAX_DIFF_FILES and we have to pick a subset. These are exactly the surfaces
  452. # single-shot and agentic reviews disagree on most (auth, routing, IPC,
  453. # subprocess, deserialization). Matched as lowercase substrings against the
  454. # path; not regex — keep it cheap.
  455. _SECURITY_RISK_PATH_TOKENS = (
  456. "auth", "login", "session", "token", "secret", "credential", "perm",
  457. "acl", "rbac", "iam", "policy",
  458. "route", "handler", "controller", "endpoint", "api/", "/api", "gateway",
  459. "middleware", "view",
  460. "exec", "subprocess", "shell", "spawn", "command",
  461. "client", "request", "fetch", "http", "url",
  462. "serialize", "pickle", "yaml", "parse", "deser",
  463. # Short tokens that would substring-match unrelated names (`format`,
  464. # `transform`, `sandbox`, `platform`) are intentionally omitted —
  465. # `sql`/`query` already cover the DB surface.
  466. "sql", "query",
  467. )
  468. # Suffixes that pass _is_reviewable_source but are almost always low-signal
  469. # in large scaffolds — generated clients, migrations, test fixtures, config
  470. # shims. These go to the BACK of the priority sort, not dropped outright.
  471. _LOW_PRIORITY_SUFFIXES = (
  472. ".gen.ts", ".gen.tsx", ".generated.ts", "_gen.py",
  473. ".test.ts", ".test.tsx", ".test.py", ".spec.ts", ".spec.js",
  474. ".config.js", ".config.ts", ".config.mjs", ".config.cjs",
  475. )
  476. _LOW_PRIORITY_PATH_TOKENS = (
  477. "/migrations/", "/alembic/versions/", "/__tests__/", "/fixtures/",
  478. )
  479. def _prioritize_diff_files(diff_files, cap):
  480. """When `diff_files` exceeds `cap`, return the top-`cap` by security
  481. relevance plus the count dropped. Otherwise return (diff_files, 0).
  482. Score = (risk_tokens_in_path, not_low_priority, added_lines). The
  483. added-lines proxy is `content.count('\\n+')` which counts diff additions
  484. cheaply without re-parsing hunks. This is a heuristic, not a guarantee —
  485. the goal is to review the likely-dangerous subset of an over-cap diff
  486. instead of reviewing nothing. Diffs that exceed the cap are typically
  487. large multi-file scaffolds, and the cross-file source→sink vulnerabilities
  488. in them concentrate in a handful of api/client/route files.
  489. """
  490. if len(diff_files) <= cap:
  491. return diff_files, 0
  492. def _score(item):
  493. fp, content = item
  494. low = fp.lower()
  495. # Prepend "/" so leading-slash patterns in _LOW_PRIORITY_PATH_TOKENS
  496. # match top-level dirs (git diff paths are repo-root-relative, e.g.
  497. # `migrations/001.py` not `/migrations/001.py`). Same trick as
  498. # _is_reviewable_source.
  499. low_slashed = "/" + low
  500. risk = sum(1 for t in _SECURITY_RISK_PATH_TOKENS if t in low)
  501. low_prio = (
  502. fp.endswith(_LOW_PRIORITY_SUFFIXES)
  503. or any(t in low_slashed for t in _LOW_PRIORITY_PATH_TOKENS)
  504. )
  505. # added_lines: count('\n+') over-counts by including '+++' header and
  506. # any literal '+' at line start in context, but it's a consistent
  507. # ordinal across files in the same diff which is all we need.
  508. added = content.count("\n+")
  509. return (risk, not low_prio, added)
  510. ranked = sorted(diff_files, key=_score, reverse=True)
  511. return ranked[:cap], len(diff_files) - cap
  512. def _is_reviewable_source(file_path):
  513. # Normalize for component matching: a path like `.next/x.js` or
  514. # `pkg/node_modules/y.ts` should both be excluded; matching against
  515. # `'/' + path` lets each pattern be checked as `'/' + p in '/' + path`
  516. # without false-positiving on `rebuild/` matching `build/`.
  517. norm = "/" + file_path.replace("\\", "/")
  518. if any(("/" + p) in norm for p in SKIP_PATH_PATTERNS):
  519. return False
  520. if file_path.endswith(SKIP_FILE_SUFFIXES):
  521. return False
  522. ext = os.path.splitext(file_path)[1].lower()
  523. if ext in SOURCE_CODE_EXTENSIONS:
  524. return True
  525. base = os.path.basename(file_path).lower()
  526. # Accept dot-suffixed variants too: `Dockerfile.dev`, `Makefile.am`,
  527. # `Jenkinsfile.release`. splitext gives ext='.dev'/'.am' for these so they
  528. # miss both the extension check and the exact-basename check otherwise.
  529. if base in SOURCE_CODE_BASENAMES \
  530. or base.split(".", 1)[0] in SOURCE_CODE_BASENAMES:
  531. return True
  532. # Extensionless files default to reviewable unless they're known
  533. # plain-text metadata or dotfiles. Covers shebang scripts under bin/ or
  534. # scripts/ (`deploy`, `run-canary`, `entrypoint`) which carry
  535. # shell-injection surface but were previously filtered out — the largest
  536. # remaining false-negative class for extensionless files. Dotfiles (`.gitignore`,
  537. # `.nvmrc`, `.env`) are config, not code; `.bashrc`-style runnables are
  538. # rare in repos and not worth the noise. The deny-list is prefix-aware on
  539. # `-`/`_` so dual-license / i18n variants (`LICENSE-MIT`, `README-CN`)
  540. # don't fall through as source.
  541. if ext == "" and not base.startswith("."):
  542. if any(base == x or base.startswith(x + "-") or base.startswith(x + "_")
  543. for x in NON_SOURCE_EXTENSIONLESS_BASENAMES):
  544. return False
  545. return True
  546. return False
  547. def extract_file_paths_from_diff(diff_output):
  548. """
  549. Extract file paths from unified diff output (without content).
  550. Only includes files with source code extensions.
  551. Returns a list of file paths.
  552. """
  553. if not diff_output or not diff_output.strip():
  554. return []
  555. paths = []
  556. file_diffs = diff_output.split("diff --git ")
  557. for file_diff in file_diffs:
  558. if not file_diff.strip():
  559. continue
  560. lines = file_diff.split('\n')
  561. header_match = re.match(r'^a/(.+?) b/(.+)$', lines[0])
  562. if not header_match:
  563. continue
  564. file_path = header_match.group(2) or header_match.group(1) or ''
  565. if not _is_reviewable_source(file_path):
  566. continue
  567. paths.append(file_path)
  568. return paths
  569. def parse_diff_into_files(diff_output):
  570. """
  571. Parse unified diff output into a list of (file_path, diff_content) tuples.
  572. Only includes files with source code extensions.
  573. """
  574. if not diff_output or not diff_output.strip():
  575. return []
  576. files = []
  577. file_diffs = diff_output.split("diff --git ")
  578. for file_diff in file_diffs:
  579. if not file_diff.strip():
  580. continue
  581. # Extract filename from first line: "a/path/to/file b/path/to/file"
  582. lines = file_diff.split('\n')
  583. header_match = re.match(r'^a/(.+?) b/(.+)$', lines[0])
  584. if not header_match:
  585. continue
  586. file_path = header_match.group(2) or header_match.group(1) or ''
  587. # Filter to source code files only
  588. if not _is_reviewable_source(file_path):
  589. continue
  590. # Extract the diff content (from first @@ onwards)
  591. diff_lines = []
  592. in_hunks = False
  593. for line in lines[1:]:
  594. if line.startswith('@@'):
  595. in_hunks = True
  596. if in_hunks:
  597. diff_lines.append(line)
  598. if diff_lines:
  599. files.append((file_path, '\n'.join(diff_lines)))
  600. return files
  601. def filter_preexisting_from_diff(diff_files, cwd, baseline_sha):
  602. """
  603. Filter out pre-existing content from diff files.
  604. When a file is fully rewritten (Write tool replaces entire content),
  605. git shows all lines as removed (-) then re-added (+). This function
  606. detects such rewrites and strips lines from the + section that also
  607. appeared in the - section, so the LLM reviewer only sees truly new code.
  608. """
  609. if not baseline_sha:
  610. return diff_files
  611. filtered = []
  612. for file_path, diff_content in diff_files:
  613. lines = diff_content.split('\n')
  614. # Collect removed and added lines (stripping the +/- prefix)
  615. removed_lines = set()
  616. added_lines = []
  617. for line in lines:
  618. if line.startswith('-') and not line.startswith('---'):
  619. removed_lines.add(line[1:].strip())
  620. elif line.startswith('+') and not line.startswith('+++'):
  621. added_lines.append(line[1:].strip())
  622. if not removed_lines:
  623. # New file, no pre-existing content to filter
  624. filtered.append((file_path, diff_content))
  625. continue
  626. # Check what fraction of added lines were pre-existing
  627. preexisting_count = sum(1 for l in added_lines if l in removed_lines)
  628. if preexisting_count == 0:
  629. filtered.append((file_path, diff_content))
  630. continue
  631. added_lines_set = set(added_lines)
  632. # Rebuild diff with pre-existing lines converted to context (space prefix).
  633. # Known imprecision: .strip() matches across indentation (so reindented
  634. # code is treated as unchanged) and the set lets one removal mask N
  635. # additions of the same stripped text. Accepted trade-off — this filter
  636. # exists for the full-file Write rewrite case where exact-match would
  637. # miss everything; the diff-review prompt's previous-findings recheck
  638. # is the backstop.
  639. new_lines = []
  640. for line in lines:
  641. if line.startswith('+') and not line.startswith('+++'):
  642. content = line[1:].strip()
  643. if content in removed_lines:
  644. # Convert to context line (pre-existing, not new)
  645. new_lines.append(' ' + line[1:])
  646. else:
  647. new_lines.append(line)
  648. elif line.startswith('-') and not line.startswith('---'):
  649. content = line[1:].strip()
  650. if content in added_lines_set:
  651. # Skip removed lines that were re-added (they become context)
  652. continue
  653. else:
  654. new_lines.append(line)
  655. else:
  656. new_lines.append(line)
  657. filtered.append((file_path, '\n'.join(new_lines)))
  658. return filtered