Skip to content

Commit 901c0bf

Browse files
authored
fix(scanner): batch rg invocations to avoid arg-length crash (audit H4) (#49)
run_rule and detect_stack passed every candidate file on one rg command line. On Windows (32767-char CreateProcess cap) this crashed at ~180 files with an uncaught OSError (WinError 206) that exited 1 — so a scanner crash masqueraded as 'findings present' and no checkpoint/report was written. detect_stack was worst: it passed ALL files regardless of per-rule globs. Fix: _batches() splits files so each rg command stays under a conservative 24000-char budget; _run_rg() runs the batches and maps OSError -> RuntimeError (caught by cmd_scan -> exit 2, the documented 'execution error'). Verified: a 400-file repo that crashed before now scans (exit 0); fixture unchanged (32). Added a _batches unit test.
1 parent e0bfa50 commit 901c0bf

2 files changed

Lines changed: 55 additions & 18 deletions

File tree

‎dsgai_scanner_tool/cli/dsgai_scan.py‎

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,41 @@ def glob_match(rel, globs):
200200
# --------------------------------------------------------------------------- #
201201
# Rule execution
202202
# --------------------------------------------------------------------------- #
203+
# Keep each rg command line well under the Windows CreateProcess cap (32767);
204+
# other platforms allow far more, but one conservative budget is simplest.
205+
_ARG_BUDGET = 24000
206+
207+
208+
def _batches(files):
209+
"""Yield lists of files whose joined path lengths stay under _ARG_BUDGET, so
210+
a single rg invocation can never exceed the OS command-line limit (audit H4)."""
211+
batch, size = [], 0
212+
for f in files:
213+
n = len(f[0]) + 1
214+
if batch and size + n > _ARG_BUDGET:
215+
yield batch
216+
batch, size = [], 0
217+
batch.append(f)
218+
size += n
219+
if batch:
220+
yield batch
221+
222+
223+
def _run_rg(rg, base_cmd, files, scan_root, rid):
224+
"""Run rg over `files` in as many batches as needed; return stdout lines."""
225+
lines = []
226+
for batch in _batches(files):
227+
cmd = base_cmd + [f[0] for f in batch]
228+
try:
229+
proc = subprocess.run(cmd, capture_output=True, text=True, cwd=scan_root)
230+
except OSError as exc: # e.g. arg list too long on a pathological path
231+
raise RuntimeError(f"rg invocation failed for {rid}: {exc}")
232+
if proc.returncode >= 2:
233+
raise RuntimeError(f"rg failed on {rid}: {proc.stderr.strip()[:200]}")
234+
lines.extend(proc.stdout.splitlines())
235+
return lines
236+
237+
203238
def run_rule(rg, rule, files, scan_root):
204239
"""Return a set of (rel_path, line) matches for one rule.
205240
@@ -208,22 +243,14 @@ def run_rule(rg, rule, files, scan_root):
208243
"""
209244
if not files:
210245
return set()
211-
cmd = [rg, "--pcre2", "--no-config", "--with-filename", "--line-number"]
246+
base = [rg, "--pcre2", "--no-config", "--with-filename", "--line-number"]
212247
if rule["classification"] == "value_bearing":
213248
# LOCATION-ONLY: rg erases the matched text before emitting anything.
214-
cmd += ["--only-matching", "--replace", ""]
215-
cmd += ["-e", rule["pcre"], "--"]
216-
cmd += [f[0] for f in files]
217-
proc = subprocess.run(cmd, capture_output=True, text=True, cwd=scan_root)
218-
if proc.returncode >= 2:
219-
raise RuntimeError(f"rg failed on {rule['id']}: {proc.stderr.strip()[:200]}")
249+
base += ["--only-matching", "--replace", ""]
250+
base += ["-e", rule["pcre"], "--"]
220251
matches = set()
221252
abs_to_rel = {f[0]: f[1] for f in files}
222-
for line in proc.stdout.splitlines():
223-
# Format: <path>:<line>:<rest>. Path is an absolute path we passed in;
224-
# split off the trailing :line:rest without touching a drive colon.
225-
rest = line
226-
# find ':<digits>:' anchor from the path we know
253+
for line in _run_rg(rg, base, files, scan_root, rule.get("id", "?")):
227254
parsed = _parse_rg_line(line, abs_to_rel)
228255
if parsed:
229256
matches.add(parsed)
@@ -245,15 +272,13 @@ def _parse_rg_line(line, abs_to_rel):
245272
def detect_stack(rg, files, scan_root):
246273
"""Return the set of detected signal categories for NOT APPLICABLE gating."""
247274
detected = set()
248-
paths = [f[0] for f in files]
249-
if not paths:
275+
if not files:
250276
return detected
251277
for cat, patterns in DETECT_SIGNALS.items():
252278
pat = "|".join(patterns)
253-
proc = subprocess.run(
254-
[rg, "--pcre2", "--no-config", "-l", "-i", "-e", pat, "--"] + paths,
255-
capture_output=True, text=True, cwd=scan_root)
256-
if proc.returncode == 0 and proc.stdout.strip():
279+
base = [rg, "--pcre2", "--no-config", "-l", "-i", "-e", pat, "--"]
280+
# Batched so a large repo can't blow the OS arg limit (audit H4).
281+
if _run_rg(rg, base, files, scan_root, f"detect:{cat}"):
257282
detected.add(cat)
258283
return detected
259284

‎dsgai_scanner_tool/tests/test_runner.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,18 @@ def test_gitignored_env_is_still_scanned(tmp_path):
363363
assert cp["controls"]["DSGAI02"] == "FAIL"
364364

365365

366+
def test_rg_arg_batching():
367+
"""Files are split into batches under the command-line budget so a large
368+
repo can't blow the OS arg limit (audit H4)."""
369+
ds = _import_cli()
370+
files = [("/some/long/abs/path/module_%04d.py" % i, "m%d.py" % i) for i in range(5000)]
371+
batches = list(ds._batches(files))
372+
assert len(batches) > 1, "5000 files should split into multiple rg batches"
373+
for b in batches:
374+
assert sum(len(f[0]) + 1 for f in b) <= ds._ARG_BUDGET or len(b) == 1
375+
assert sum(len(b) for b in batches) == len(files) # no file dropped
376+
377+
366378
def test_rules_json_in_sync():
367379
from_yaml = yaml.safe_load(open(RULES_YAML, encoding="utf-8"))
368380
rebuilt = json.dumps(from_yaml, indent=2, sort_keys=True, ensure_ascii=False) + "\n"

0 commit comments

Comments
 (0)