From 2145cffeaeafad3af536245a3df00baef2b2c171 Mon Sep 17 00:00:00 2001 From: Scott Severance Date: Sat, 29 Aug 2026 19:02:13 +0000 Subject: [PATCH 1/2] fix: add JSON validation for truncated/malformed LLM responses --- vulntester/llm/analyzer.py | 51 +++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/vulntester/llm/analyzer.py b/vulntester/llm/analyzer.py index 21144bf..9419ccc 100644 --- a/vulntester/llm/analyzer.py +++ b/vulntester/llm/analyzer.py @@ -262,32 +262,49 @@ def _analyze_findings_batch(self, findings: list[Finding]) -> list[dict]: m = re.search(r"```(?:json)?\s*\n?(.*?)\n?```", response, re.DOTALL) if m: json_text = m.group(1) + + if not json_text or not json_text.strip(): + logger.error("JSON extraction yielded empty string for findings batch") + results.append({ + "finding_title": "Batch Analysis", + "error": "LLM response contained no valid JSON", + }) + continue + parsed = json.loads(json_text) - if isinstance(parsed, list): - if not parsed: - logger.warning("LLM returned empty JSON array for findings batch") - results.append({ - "finding_title": "Batch Analysis", - "error": "LLM returned empty array", - }) - else: - for item in parsed: - idx = item.get("finding_index", 0) - 1 - if 0 <= idx < len(batch): - item["finding_title"] = batch[idx].title - results.extend(parsed) - else: + + if not isinstance(parsed, list): logger.error(f"LLM JSON response is not an array: {type(parsed)}") results.append({ "finding_title": "Batch Analysis", "error": "LLM response JSON is not an array", }) + continue + + if not parsed: + logger.warning("LLM returned empty JSON array for findings batch") + results.append({ + "finding_title": "Batch Analysis", + "error": "LLM returned empty array", + }) + continue + + for item in parsed: + if not isinstance(item, dict): + logger.warning(f"Skipping non-dict item in LLM JSON array: {type(item)}") + continue + idx = item.get("finding_index", 0) - 1 + if 0 <= idx < len(batch): + item["finding_title"] = batch[idx].title + results.append(item) + except (json.JSONDecodeError, ValueError) as e: - logger.error(f"Failed to parse LLM JSON response: {str(e)}") - # If JSON parsing fails, store raw text + logger.error(f"Failed to parse LLM JSON response: {str(e)[:100]}") + # If JSON parsing fails, store raw text with error context results.append({ "finding_title": "Batch Analysis", - "raw_analysis": response, + "error": f"JSON parse failed: {type(e).__name__}", + "raw_analysis": response[:500], }) return results From d611e45cbb6dd865a94ccd0cc9a84c62b520b37c Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 19:06:40 +0000 Subject: [PATCH 2/2] fix(review): keep unparseable LLM responses, mark dropped batches, coerce finding_index - raw_analysis is capped at a named _MAX_RAW_ANALYSIS_CHARS (4000) with an explicit "...[truncated, N chars total]" marker and the original length logged, instead of a silent 500-char clip. - A JSON array whose elements are all non-dicts now records a "Batch Analysis" error entry rather than contributing nothing at all. - finding_index is coerced with int(), so a string index no longer raises TypeError out of _analyze_findings_batch and aborts analyze_report. - Log the full parse error; drop trailing whitespace. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_llm_analyzer.py | 85 +++++++++++++++++++++++++++++++++++++- vulntester/llm/analyzer.py | 64 ++++++++++++++++++++++++---- 2 files changed, 139 insertions(+), 10 deletions(-) diff --git a/tests/test_llm_analyzer.py b/tests/test_llm_analyzer.py index 29e3b4d..c713d7d 100644 --- a/tests/test_llm_analyzer.py +++ b/tests/test_llm_analyzer.py @@ -15,7 +15,7 @@ import pytest from vulntester.config import Severity -from vulntester.llm.analyzer import LLMAnalyzer +from vulntester.llm.analyzer import _MAX_RAW_ANALYSIS_CHARS, LLMAnalyzer from vulntester.llm.client import LLMClient, LLMError, LLMProvider from vulntester.report.models import Finding, ScanReport, ZeroDayAnomaly @@ -191,6 +191,89 @@ def test_analyzer_uses_the_client_request_timeout(analyzer): assert kwargs["timeout"] == LLMClient._REQUEST_TIMEOUT +# --- a malformed array must not vanish, and must not abort the run ----------- + + +def _responding(text: str): + return lambda *a, **k: _FakeResponse(_claude_text(text)) + + +def test_array_of_only_non_dicts_records_an_error(analyzer): + """Every element skipped used to append nothing at all.""" + payload = json.dumps(["finding 1 is fine", "finding 2 is fine"]) + with patch("urllib.request.urlopen", side_effect=_responding(payload)): + results = analyzer._analyze_findings_batch([_finding()]) + + assert len(results) == 1 + assert "no analysis objects" in results[0]["error"] + + +def test_non_dict_elements_are_skipped_without_flagging_a_usable_batch(analyzer): + payload = json.dumps(["noise", {"finding_index": 1, "risk_rating": 4}]) + with patch("urllib.request.urlopen", side_effect=_responding(payload)): + results = analyzer._analyze_findings_batch([_finding()]) + + assert results == [ + {"finding_index": 1, "risk_rating": 4, "finding_title": "Finding 1"} + ] + + +def test_empty_json_extraction_records_an_error(analyzer): + with patch("urllib.request.urlopen", side_effect=_responding("```json\n\n```")): + results = analyzer._analyze_findings_batch([_finding()]) + + assert len(results) == 1 + assert "no valid JSON" in results[0]["error"] + + +def test_string_finding_index_is_coerced_not_raised(analyzer): + """``"1" - 1`` raises TypeError, which the except clause does not catch.""" + payload = json.dumps([{"finding_index": "1", "risk_rating": 6}]) + with patch("urllib.request.urlopen", side_effect=_responding(payload)): + results = analyzer._analyze_findings_batch([_finding()]) + + assert results == [ + {"finding_index": "1", "risk_rating": 6, "finding_title": "Finding 1"} + ] + + +def test_unusable_finding_index_leaves_the_item_untitled(analyzer): + payload = json.dumps([{"finding_index": None, "risk_rating": 6}]) + with patch("urllib.request.urlopen", side_effect=_responding(payload)): + results = analyzer._analyze_findings_batch([_finding()]) + + assert results == [{"finding_index": None, "risk_rating": 6}] + + +def test_a_bad_index_does_not_discard_the_rest_of_the_report(analyzer): + payload = json.dumps([{"finding_index": "1", "risk_rating": 6}]) + with patch("urllib.request.urlopen", side_effect=_responding(payload)): + results = analyzer.analyze_report(_report()) + + assert results["finding_analysis"][0]["risk_rating"] == 6 + + +# --- an unparseable response must not be silently clipped -------------------- + + +def test_short_unparseable_response_is_kept_whole(analyzer): + text = "Sorry, I cannot produce JSON for these findings." + with patch("urllib.request.urlopen", side_effect=_responding(text)): + results = analyzer._analyze_findings_batch([_finding()]) + + assert results[0]["raw_analysis"] == text + + +def test_long_unparseable_response_is_marked_as_truncated(analyzer): + text = "x" * (_MAX_RAW_ANALYSIS_CHARS + 5000) + with patch("urllib.request.urlopen", side_effect=_responding(text)): + results = analyzer._analyze_findings_batch([_finding()]) + + raw = results[0]["raw_analysis"] + assert raw.startswith("x" * _MAX_RAW_ANALYSIS_CHARS) + assert f"...[truncated, {len(text)} chars total]" in raw + + # --- successful paths still work -------------------------------------------- diff --git a/vulntester/llm/analyzer.py b/vulntester/llm/analyzer.py index 9419ccc..0566403 100644 --- a/vulntester/llm/analyzer.py +++ b/vulntester/llm/analyzer.py @@ -13,6 +13,41 @@ # Maximum findings to send per LLM request to avoid token limits _MAX_FINDINGS_PER_BATCH = 25 +# Cap on the unparseable response text kept for the report. A batch covers up to +# _MAX_FINDINGS_PER_BATCH findings, so the raw text can be many kilobytes; the cap +# keeps the report readable and always leaves a marker saying what was dropped. +_MAX_RAW_ANALYSIS_CHARS = 4000 + + +def _raw_analysis_text(response: str) -> str: + """Return ``response`` for the report, marked and logged if it had to be cut.""" + if len(response) <= _MAX_RAW_ANALYSIS_CHARS: + return response + logger.error( + "Unparseable LLM response truncated for the report: kept %d of %d chars", + _MAX_RAW_ANALYSIS_CHARS, + len(response), + ) + return ( + response[:_MAX_RAW_ANALYSIS_CHARS] + + f"\n...[truncated, {len(response)} chars total]" + ) + + +def _finding_index(item: dict) -> int: + """Return the 1-based ``finding_index``, or 0 when it is missing or not a number. + + Models sometimes emit the index as a string (``"finding_index": "1"``). + Subtracting from that raises ``TypeError``, which would abort the whole run, + so coerce here and let one bad element degrade to "untitled" instead. + """ + raw = item.get("finding_index", 0) + try: + return int(raw) + except (TypeError, ValueError): + logger.warning("Ignoring non-numeric finding_index: %r", raw) + return 0 + class LLMAnalyzer: """Uses an LLM to analyze scan results and provide detailed remediation guidance.""" @@ -262,7 +297,7 @@ def _analyze_findings_batch(self, findings: list[Finding]) -> list[dict]: m = re.search(r"```(?:json)?\s*\n?(.*?)\n?```", response, re.DOTALL) if m: json_text = m.group(1) - + if not json_text or not json_text.strip(): logger.error("JSON extraction yielded empty string for findings batch") results.append({ @@ -270,9 +305,9 @@ def _analyze_findings_batch(self, findings: list[Finding]) -> list[dict]: "error": "LLM response contained no valid JSON", }) continue - + parsed = json.loads(json_text) - + if not isinstance(parsed, list): logger.error(f"LLM JSON response is not an array: {type(parsed)}") results.append({ @@ -280,7 +315,7 @@ def _analyze_findings_batch(self, findings: list[Finding]) -> list[dict]: "error": "LLM response JSON is not an array", }) continue - + if not parsed: logger.warning("LLM returned empty JSON array for findings batch") results.append({ @@ -288,23 +323,34 @@ def _analyze_findings_batch(self, findings: list[Finding]) -> list[dict]: "error": "LLM returned empty array", }) continue - + + analyzed = 0 for item in parsed: if not isinstance(item, dict): logger.warning(f"Skipping non-dict item in LLM JSON array: {type(item)}") continue - idx = item.get("finding_index", 0) - 1 + idx = _finding_index(item) - 1 if 0 <= idx < len(batch): item["finding_title"] = batch[idx].title results.append(item) - + analyzed += 1 + + if not analyzed: + # Every element was skipped: without a marker the caller would + # see the batch simply vanish. + logger.error("LLM JSON array held no analysis objects for findings batch") + results.append({ + "finding_title": "Batch Analysis", + "error": "LLM response JSON array contained no analysis objects", + }) + except (json.JSONDecodeError, ValueError) as e: - logger.error(f"Failed to parse LLM JSON response: {str(e)[:100]}") + logger.error(f"Failed to parse LLM JSON response: {e}") # If JSON parsing fails, store raw text with error context results.append({ "finding_title": "Batch Analysis", "error": f"JSON parse failed: {type(e).__name__}", - "raw_analysis": response[:500], + "raw_analysis": _raw_analysis_text(response), }) return results