Skip to content

Commit a8c54c9

Browse files
committed
fix(engine): surface scanner exceptions as failure rows in canonical results
Both ``_run_sequential`` and ``_run_parallel`` previously caught exceptions raised from ``scanner.scan()`` and silently dropped the scanner from the result list. Other scanners that failed via the ``_run_in_container`` path produced an ``execution_failed`` ScanResult row visible in argus-results.json; scanners with custom ``scan()`` implementations that raise (FileNotFoundError when a binary is missing, RuntimeError, TimeoutExpired, etc.) disappeared entirely. Concretely: ``lint-dockerfile`` (HadolintLinter) calls ``subprocess.run(['hadolint', ...])`` directly. When hadolint is not installed locally and the container backend is unavailable, the FileNotFoundError propagates up through scan() — the engine logged the exception and moved on, leaving no trace in canonical results. The user looking at argus-results.json had no signal that lint-dockerfile was even attempted, much less why it didn't run. Per ADR-016: silent failures are the anti-pattern. The fix: * New ``_failure_result(scanner_name, exc, duration_ms=None)`` helper at the top of ``argus/core/engine.py`` builds a ScanResult with the same ``execution_failed`` and ``execution_failure_reason`` metadata shape ``_run_in_container`` already emits for output-less docker runs. * ``_run_sequential`` (line 304) appends the failure row before the fail-fast check, so even with --fail-fast the scanner that broke the loop is visible in results. * ``_run_parallel`` (line 389) does the same when collecting futures. Tests updated: * ``test_run_handles_scanner_exception``, ``test_local_backend_fails_if_unavailable``, ``test_docker_backend_no_image_raises``, ``test_timeout_raises_on_slow_scanner``, ``test_version_mismatch_raises_by_default``, ``test_auto_backend_local_fallback_checks_version``, ``test_run_skips_unavailable_scanners`` (renamed), ``test_fail_fast_aborts_after_failure``, ``test_without_fail_fast_continues_after_failure`` — all updated from ``len(results) == 0`` (silent drop) to asserting the ``execution_failed`` failure row. * New ``test_parallel_failure_surfaces_as_failure_row`` — regression test for the lint-dockerfile bug specifically: scanner raises FileNotFoundError in parallel mode, failure row appears in canonical results with the exception type captured. 1517 SDK tests pass.
1 parent 0bf1a26 commit a8c54c9

2 files changed

Lines changed: 113 additions & 18 deletions

File tree

argus/core/engine.py

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,31 @@
2525
}
2626

2727

28+
def _failure_result(
29+
scanner_name: str,
30+
exc: BaseException,
31+
duration_ms: int | None = None,
32+
) -> ScanResult:
33+
"""Build a ScanResult representing a scanner that raised during execution.
34+
35+
Mirrors the ``execution_failed`` metadata that ``_run_in_container``
36+
produces for output-less docker runs, so the canonical results
37+
contract is uniform regardless of which path produced the failure.
38+
A user reviewing argus-results.json sees the scanner with its
39+
error reason; without this, scanners whose ``scan()`` raises (e.g.
40+
a missing local binary that subprocess.run can't find) silently
41+
disappear from the results — exactly the silent-failure pattern
42+
ADR-016 was written to prevent.
43+
"""
44+
metadata: dict = {
45+
"execution_failed": True,
46+
"execution_failure_reason": f"{type(exc).__name__}: {exc}",
47+
}
48+
if duration_ms is not None:
49+
metadata["duration_ms"] = duration_ms
50+
return ScanResult(scanner=scanner_name, metadata=metadata)
51+
52+
2853
class ArgusEngine:
2954
"""Orchestrates registered scanners and aggregates their results."""
3055

@@ -312,11 +337,18 @@ def _run_sequential(
312337
scanner.name, elapsed, result.total_count,
313338
)
314339
results.append(result)
315-
except Exception:
340+
except Exception as exc:
316341
elapsed = int((time.monotonic() - start) * 1000)
317342
logger.exception(
318343
"Scanner '%s' failed after %dms", scanner.name, elapsed,
319344
)
345+
# Append a failure-row ScanResult so the user sees the
346+
# scanner in the canonical results — silently dropping
347+
# it makes a hard failure look identical to "ran clean
348+
# with zero findings". Mirrors the execution_failed
349+
# metadata that ``_run_in_container`` produces for
350+
# output-less docker runs.
351+
results.append(_failure_result(scanner.name, exc, elapsed))
320352
if fail_fast:
321353
logger.error(
322354
"Aborting scan — --fail-fast is set and '%s' failed",
@@ -386,8 +418,12 @@ def _timed_run(scanner, scan_path, config_dict, patterns):
386418
result.total_count,
387419
)
388420
results.append(result)
389-
except Exception:
421+
except Exception as exc:
390422
logger.exception("Scanner '%s' failed", name)
423+
# See _run_sequential for rationale — failure rows
424+
# surface in canonical results instead of silently
425+
# dropping the scanner.
426+
results.append(_failure_result(name, exc))
391427
if fail_fast:
392428
logger.error(
393429
"Aborting scan — --fail-fast and '%s' failed",

argus/tests/test_engine.py

Lines changed: 75 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,19 @@ def test_run_skips_disabled_scanners(self):
100100
assert len(summary.results) == 1
101101
assert summary.results[0].scanner == "bandit"
102102

103-
def test_run_skips_unavailable_scanners(self):
103+
def test_run_surfaces_unavailable_scanner_as_failure_row(self):
104+
"""A scanner the user enabled in config but that isn't installed
105+
locally surfaces as a failure row, not a silent skip — the user
106+
explicitly asked for it, so they should see why it didn't run.
107+
"""
104108
engine = self._make_engine(
105109
scanners_config={"bandit": {"enabled": True}}
106110
)
107111
engine.register_scanner(MockScanner("bandit", available=False))
108112

109113
summary = engine.run()
110-
assert len(summary.results) == 0
114+
assert len(summary.results) == 1
115+
assert summary.results[0].metadata.get("execution_failed") is True
111116

112117
def test_run_skips_unregistered_scanners(self):
113118
engine = self._make_engine()
@@ -177,7 +182,11 @@ def install_command(self):
177182

178183
engine.register_scanner(FailingScanner())
179184
summary = engine.run()
180-
assert len(summary.results) == 0
185+
# Failure row surfaces in canonical results (ADR-016) — silent
186+
# drops were the bug behind ``lint-dockerfile`` going missing.
187+
assert len(summary.results) == 1
188+
assert summary.results[0].metadata.get("execution_failed") is True
189+
assert summary.results[0].total_count == 0
181190

182191

183192
class TestDockerExecutionBackend:
@@ -207,9 +216,11 @@ def test_local_backend_fails_if_unavailable(self):
207216
scanner = MockScanner("bandit", available=False)
208217
engine.register_scanner(scanner)
209218

210-
# Engine catches exceptions and logs them
219+
# Engine surfaces the failure as a row with execution_failed
220+
# metadata (ADR-016 — no silent drops).
211221
summary = engine.run(scanner_names=["bandit"])
212-
assert len(summary.results) == 0
222+
assert len(summary.results) == 1
223+
assert summary.results[0].metadata.get("execution_failed") is True
213224

214225
def test_resolve_image_no_registry(self):
215226
engine = self._make_engine(registry="")
@@ -238,7 +249,8 @@ def test_docker_backend_no_image_raises(self):
238249
engine.register_scanner(scanner)
239250

240251
summary = engine.run(scanner_names=["bandit"])
241-
assert len(summary.results) == 0
252+
assert len(summary.results) == 1
253+
assert summary.results[0].metadata.get("execution_failed") is True
242254

243255
def test_auto_backend_prefers_container(self, monkeypatch):
244256
"""auto backend uses containers first when Docker is available."""
@@ -325,8 +337,13 @@ def install_command(self):
325337
engine.register_scanner(good)
326338

327339
summary = engine.run(fail_fast=True, parallel=False)
328-
# Scanner "a" fails, "b" should never run (sequential mode)
329-
assert len(summary.results) == 0
340+
# Scanner "a" produces a failure row (recorded for visibility)
341+
# then the loop breaks before running "b". No silent drop.
342+
assert len(summary.results) == 1
343+
failed = summary.results[0]
344+
assert failed.scanner == "a"
345+
assert failed.metadata.get("execution_failed") is True
346+
assert "boom" in failed.metadata.get("execution_failure_reason", "")
330347
assert good.scan_called_with is None
331348

332349
def test_without_fail_fast_continues_after_failure(self):
@@ -348,9 +365,13 @@ def install_command(self):
348365
engine.register_scanner(good)
349366

350367
summary = engine.run(fail_fast=False)
351-
# Scanner "a" fails, "b" still runs
352-
assert len(summary.results) == 1
368+
# Both scanners present in canonical results: "a" as a
369+
# failure row, "b" as a normal success row.
370+
assert len(summary.results) == 2
353371
assert good.scan_called_with is not None
372+
by_name = {r.scanner: r for r in summary.results}
373+
assert by_name["a"].metadata.get("execution_failed") is True
374+
assert by_name["b"].metadata.get("execution_failed") is None
354375

355376

356377
class TestParallelExecution:
@@ -371,6 +392,40 @@ def test_parallel_runs_all_scanners(self):
371392
summary = engine.run(parallel=True)
372393
assert len(summary.results) == 3
373394

395+
def test_parallel_failure_surfaces_as_failure_row(self):
396+
"""Regression for ADR-016: a scanner that raises in parallel mode
397+
produces a ScanResult with execution_failed metadata, not a
398+
silent drop. This is the bug behind ``lint-dockerfile`` going
399+
missing from results when hadolint isn't installed locally —
400+
custom scan() implementations that raise FileNotFoundError used
401+
to disappear from canonical results entirely.
402+
"""
403+
engine = self._make_engine(2) # config has scanners s0, s1
404+
405+
class FailingScanner:
406+
name = "s0"
407+
def scan(self, path, config=None):
408+
raise FileNotFoundError(2, "No such file", "broken-tool")
409+
def is_available(self):
410+
return True
411+
def install_command(self):
412+
return None
413+
414+
good = MockScanner("s1", findings=[
415+
Finding(id="1", severity=Severity.LOW, title="f"),
416+
])
417+
engine.register_scanner(FailingScanner())
418+
engine.register_scanner(good)
419+
420+
summary = engine.run(parallel=True)
421+
assert len(summary.results) == 2
422+
by_name = {r.scanner: r for r in summary.results}
423+
assert by_name["s0"].metadata.get("execution_failed") is True
424+
assert "FileNotFoundError" in by_name["s0"].metadata.get(
425+
"execution_failure_reason", "",
426+
)
427+
assert by_name["s1"].metadata.get("execution_failed") is None
428+
374429
def test_parallel_faster_than_sequential(self):
375430
"""Parallel should be faster when scanners have I/O wait."""
376431
import time as time_mod
@@ -463,8 +518,9 @@ def install_command(self):
463518

464519
engine.register_scanner(SlowScanner())
465520
summary = engine.run(timeout=1)
466-
# Scanner should time out and produce no results
467-
assert len(summary.results) == 0
521+
# Timeout surfaces as a failure row, not a silent drop.
522+
assert len(summary.results) == 1
523+
assert summary.results[0].metadata.get("execution_failed") is True
468524

469525
def test_no_timeout_allows_completion(self):
470526
engine = self._make_engine()
@@ -1399,9 +1455,11 @@ def test_version_mismatch_raises_by_default(self, monkeypatch):
13991455
lambda name: "1.0.0",
14001456
)
14011457

1402-
# Engine catches exceptions — scanner produces no results
1458+
# Version mismatch surfaces as a failure row instead of a
1459+
# silent drop.
14031460
summary = engine.run(scanner_names=["bandit"])
1404-
assert len(summary.results) == 0
1461+
assert len(summary.results) == 1
1462+
assert summary.results[0].metadata.get("execution_failed") is True
14051463

14061464
def test_version_mismatch_allowed_with_flag(self, monkeypatch):
14071465
"""With allow_local_versions=True, mismatch logs warning but proceeds."""
@@ -1477,8 +1535,9 @@ def test_auto_backend_local_fallback_checks_version(self, monkeypatch):
14771535
)
14781536

14791537
summary = engine.run(scanner_names=["bandit"])
1480-
# Version mismatch should cause failure
1481-
assert len(summary.results) == 0
1538+
# Version mismatch surfaces as a failure row.
1539+
assert len(summary.results) == 1
1540+
assert summary.results[0].metadata.get("execution_failed") is True
14821541

14831542
def test_tool_version_recorded_in_metadata(self, monkeypatch):
14841543
"""Tool version should be recorded in result metadata."""

0 commit comments

Comments
 (0)