From 4f11fad9ac1f272c025b866ec36842be6fa35074 Mon Sep 17 00:00:00 2001 From: Mohit Gupta Date: Wed, 4 Nov 2026 11:07:50 +0530 Subject: [PATCH 1/4] fix(llm): report which check rejected a model answer Ledger rows for a rejected structured response all said ValidationError, which hid whether the model answered in prose, dropped a required field, returned the wrong tool, or was cut short. Tag every rejection with a fixed check code and add a content-free diagnostic to the ledger row and log line: the check, the answer channel (tool call or text), the provider's completion label, where prose or code fences surrounded a JSON object, schema field paths with their validation error types, and the check that rejected each bounded attempt. Diagnostics are validated against a fixed grammar and never carry model output, field values or undeclared keys. Backport of the diagnostics change prepared for main; retries, recovery and fail-closed outcomes are unchanged. Signed-off-by: Mohit Gupta Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + docs/scan-completeness.md | 2 +- src/skillspector/inspection_ledger.py | 32 ++ src/skillspector/llm_analyzer_base.py | 84 ++++- src/skillspector/llm_utils.py | 54 ++- .../nodes/analyzers/mcp_tool_poisoning.py | 1 + src/skillspector/nodes/meta_analyzer.py | 1 + src/skillspector/structured_response.py | 307 ++++++++++++++++-- tests/test_mcp_tool_poisoning.py | 11 +- tests/unit/test_structured_response.py | 144 +++++++- 10 files changed, 590 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce0c19a94..da419aa16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ### Unreleased maintenance fixes ### Fixes * Backport scoped `allowed-tools` grant handling from #768: recognize declarations such as `Bash(git:*)`, preserve spaces and commas inside scopes, and keep malformed grants or embedded tool names from declaring shell capability. Command restrictions remain intact in the manifest. +* Report which check rejected a model answer. Ledger rows and logs for rejected structured responses carry a `diagnostic` built only from fixed codes (check, channel, completion label, JSON shape, schema paths and per-attempt checks), never model output. --- ### 2.13.3 (Unreleased) ### Security and integrity fixes diff --git a/docs/scan-completeness.md b/docs/scan-completeness.md index 65e18bbdf..1069fa5d6 100644 --- a/docs/scan-completeness.md +++ b/docs/scan-completeness.md @@ -129,7 +129,7 @@ The examples below use static-only analysis, no baseline suppression, and benign Fatal execution failure takes precedence and exits 2 regardless of strict flags. Otherwise, the CLI exits 1 for requested incomplete/findings gates or a risk score above 50. Thus **default CLI exit 0 is not an installation-safety verdict**. MCP independently requires successful execution, complete analysis, zero entirely uninspected files, a score at most 50, and fulfillment of any requested LLM analysis. The table explicitly requests `use_llm=false`; default LLM requests have an additional requirement. -JSON and MCP expose `analysis_completeness.status`, `is_complete`, `ledger_exceptions`, `scope_exclusions`, `analyzer_statuses`, and `limitations`, as well as `execution_successful`. Exceptions include reason, message, path, fatality, available source lines, and applicable limit metrics. Terminal and Markdown reports show these projections; SARIF uses invocation completeness and notifications. The report raises an otherwise `SAFE` recommendation to `CAUTION` for incomplete analysis while retaining the actual score and severity. +JSON and MCP expose `analysis_completeness.status`, `is_complete`, `ledger_exceptions`, `scope_exclusions`, `analyzer_statuses`, and `limitations`, as well as `execution_successful`. Exceptions include reason, message, path, fatality, available source lines, and applicable limit metrics. An exception for a rejected model answer also carries a `diagnostic` built only from fixed codes: the check that rejected the answer (for example `not_json_object` or `invalid_field`), whether it arrived as a tool call or text, the provider's completion label, where prose or code fences surrounded a JSON object, schema field paths with their validation error types, and the check that rejected each bounded attempt, such as `check=not_json_object; channel=text; finish=stop; shape=prefix+fence; attempts=4; checks=not_json_object,not_json_object,not_json_object,not_json_object`. It never contains model output, field values, or keys the schema does not declare, which are reported as `*`. Terminal and Markdown reports show these projections; SARIF uses invocation completeness and notifications. The report raises an otherwise `SAFE` recommendation to `CAUTION` for incomplete analysis while retaining the actual score and severity. A subtlety is that component coverage can still read **100%** for AE6: analyzer work completed on each file, while a separate system event records unresolved interpretation. Consumers must use `is_complete` and the ledger, not the coverage percentage alone. No new telemetry platform is needed to explain these decisions; the existing public fields distinguish the causes. diff --git a/src/skillspector/inspection_ledger.py b/src/skillspector/inspection_ledger.py index 7b936858c..90c4bd5d9 100644 --- a/src/skillspector/inspection_ledger.py +++ b/src/skillspector/inspection_ledger.py @@ -6,6 +6,7 @@ from __future__ import annotations import logging +import re from collections.abc import Callable, Iterable, Mapping from enum import StrEnum from hashlib import sha256 @@ -18,6 +19,12 @@ MAX_INSPECTION_LEDGER_EVENTS: Final = 10_000 MAX_FINDING_OUTPUT_RECORDS: Final = 10_000 MAX_EFFECTIVE_FINDINGS: Final = MAX_FINDING_OUTPUT_RECORDS +# Fixed codes, counts and schema field paths only (``check=invalid_field; +# schema=findings.0.severity:literal_error``); anything else is dropped. +MAX_DIAGNOSTIC_CHARACTERS: Final = 512 +_DIAGNOSTIC: Final = re.compile( + r"[a-z0-9_]+=[A-Za-z0-9_.:*$+,-]+(?:; [a-z0-9_]+=[A-Za-z0-9_.:*$+,-]+)*" +) class LedgerOutcome(StrEnum): @@ -314,6 +321,7 @@ class InspectionLedgerEvent(TypedDict): reason_code: NotRequired[LedgerReason] message: NotRequired[str] error_class: NotRequired[str] + diagnostic: NotRequired[str] stage: NotRequired[str] observed_characters: NotRequired[int] limit_characters: NotRequired[int] @@ -355,6 +363,7 @@ class InspectionLedgerException(TypedDict): start_line: int | None end_line: int | None error_class: NotRequired[str] + diagnostic: NotRequired[str] analyzers: NotRequired[list[str]] fatal: NotRequired[bool] scope: NotRequired[str] @@ -438,6 +447,19 @@ def _is_meta_phase(phase: str) -> bool: return phase == "meta" +def _safe_diagnostic(diagnostic: object) -> str | None: + """Keep a bounded diagnostic of fixed codes; drop anything that could carry content.""" + if ( + isinstance(diagnostic, str) + and len(diagnostic) <= MAX_DIAGNOSTIC_CHARACTERS + and _DIAGNOSTIC.fullmatch(diagnostic) + ): + return diagnostic + if diagnostic is not None: + logger.debug("Dropping a ledger diagnostic outside the fixed-code format") + return None + + def ledger_event( *, outcome: LedgerOutcome, @@ -451,6 +473,7 @@ def ledger_event( input_finding_ids: Iterable[str] = (), emitted_finding_ids: Iterable[str] = (), error_class: str | None = None, + diagnostic: str | None = None, stage: str | None = None, observed_characters: int | None = None, limit_characters: int | None = None, @@ -518,6 +541,9 @@ def ledger_event( event["message"] = REASON_MESSAGES[reason] if error_class is not None: event["error_class"] = error_class + safe_diagnostic = _safe_diagnostic(diagnostic) + if safe_diagnostic is not None: + event["diagnostic"] = safe_diagnostic if stage is not None: event["stage"] = stage if observed_characters is not None: @@ -650,6 +676,7 @@ def _exception( start_line: int | None = None, end_line: int | None = None, error_class: str | None = None, + diagnostic: str | None = None, analyzers: Iterable[str] = (), fatal: bool, scope: str | None = None, @@ -670,6 +697,9 @@ def _exception( exception["analyzers"] = analyzer_ids if error_class: exception["error_class"] = error_class + safe_diagnostic = _safe_diagnostic(diagnostic) + if safe_diagnostic is not None: + exception["diagnostic"] = safe_diagnostic if scope: exception["scope"] = scope return exception @@ -698,6 +728,7 @@ def _exception_from_event( start_line=event.get("start_line"), end_line=event.get("end_line"), error_class=event.get("error_class"), + diagnostic=event.get("diagnostic"), analyzers=[str(event.get("analyzer_id", ""))], fatal=fatal, scope=RULE_SET_SCOPE if _is_rule_set_record(event) else None, @@ -719,6 +750,7 @@ def _merge_exception_projection( exception["start_line"], exception["end_line"], exception.get("error_class"), + exception.get("diagnostic"), # A rule-set row and a real file's row can share a path label; # they must never merge into one public row. exception.get("scope"), diff --git a/src/skillspector/llm_analyzer_base.py b/src/skillspector/llm_analyzer_base.py index 4515bde28..d089eb4a4 100644 --- a/src/skillspector/llm_analyzer_base.py +++ b/src/skillspector/llm_analyzer_base.py @@ -72,7 +72,7 @@ from skillspector.logging_config import get_logger from skillspector.model_info import get_max_input_tokens from skillspector.models import Finding -from skillspector.structured_response import ResponseIssue +from skillspector.structured_response import ResponseCheck, ResponseIssue, schema_error_paths logger = get_logger(__name__) @@ -123,6 +123,8 @@ class _StructuredResponseValidationError(Exception): def __init__(self, issue: ResponseIssue | None = None) -> None: super().__init__("model response failed validation") + # The check that rejected each bounded attempt for this batch, oldest first. + self.attempt_checks: tuple[str, ...] = () self.reason = { ResponseIssue.REFUSED: LedgerReason.LLM_RESPONSE_REFUSED, ResponseIssue.TRUNCATED: LedgerReason.LLM_RESPONSE_TRUNCATED, @@ -135,6 +137,55 @@ def __init__(self, issue: ResponseIssue | None = None) -> None: ResponseIssue.FORMAT_CHANGED: LedgerReason.LLM_FORMAT_CHANGED, }.get(issue or ResponseIssue.INVALID_JSON, LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID) + @property + def check(self) -> str: + """Fixed code for the validation check that rejected this attempt.""" + cause = self.__cause__ + if isinstance(cause, StructuredOutputParseError): + return str(cause.check or cause.issue) + if isinstance(cause, ValidationError): + # A LangChain parser validated the arguments before our shared boundary. + missing = any(error.get("type") == "missing" for error in cause.errors()) + return ResponseCheck.MISSING_FIELD if missing else ResponseCheck.INVALID_FIELD + return "unknown" + + def diagnostic(self, schema: object) -> str | None: + """Describe the rejection with fixed codes and schema field paths only. + + No response text, field values or keys the schema does not declare are + included, so the result is safe to retain in reports and logs. + """ + try: + cause = self.__cause__ + parts = [f"check={self.check}"] + schema_errors: tuple[str, ...] = () + if isinstance(cause, StructuredOutputParseError): + parts.extend( + f"{name}={value}" + for name, value in ( + ("channel", cause.channel), + ("finish", cause.finish), + ("shape", cause.shape), + ) + if value + ) + schema_errors = cause.schema_errors + elif ( + isinstance(cause, ValidationError) + and isinstance(schema, type) + and issubclass(schema, BaseModel) + ): + schema_errors = schema_error_paths(cause, schema) + if schema_errors: + parts.append("schema=" + ",".join(schema_errors)) + if self.attempt_checks: + parts.append(f"attempts={len(self.attempt_checks)}") + parts.append("checks=" + ",".join(self.attempt_checks)) + return "; ".join(parts) + except Exception: + # A diagnostic must never replace the validation failure it describes. + return None + def _require_structured_response(response: object) -> None: """Reject a structured-output call that produced no result. @@ -146,14 +197,18 @@ def _require_structured_response(response: object) -> None: schema object. """ if response is None: - raise StructuredOutputParseError("model returned no structured response") + raise StructuredOutputParseError( + "model returned no structured response", check=ResponseCheck.NO_STRUCTURED_RESPONSE + ) if ( isinstance(response, BaseModel) and "findings" in type(response).model_fields and "findings" not in response.model_fields_set ): raise StructuredOutputParseError( - "response omits findings", issue=ResponseIssue.MISSING_FIELDS + "response omits findings", + issue=ResponseIssue.MISSING_FIELDS, + check=ResponseCheck.MISSING_FINDINGS, ) @@ -713,6 +768,8 @@ class BatchFailure: batch: Batch error_class: str reason: LedgerReason = LedgerReason.LLM_BATCH_FAILED + # Fixed validation codes for a rejected structured answer; never response text. + diagnostic: str | None = None @dataclass @@ -813,6 +870,7 @@ def ledger_events_for_batches( end_line=end_line, reason=failure.reason, error_class=failure.error_class, + diagnostic=failure.diagnostic, ) ) @@ -1173,7 +1231,9 @@ def _formatting_call(self, source: str) -> tuple[object, str]: schema_tokens = estimate_tokens(json.dumps(schema.model_json_schema())) if estimate_tokens(prompt) + schema_tokens > self._input_budget: raise StructuredOutputParseError( - "formatting input exceeds model budget", issue=ResponseIssue.LIMIT + "formatting input exceeds model budget", + issue=ResponseIssue.LIMIT, + check=ResponseCheck.FORMAT_INPUT_LIMIT, ) llm, _ = self._model_for_call() return bind_structured_output( @@ -1219,12 +1279,15 @@ def _invoke_batch_with_retries( structured_retries = 0 connection_retries = 0 format_source: str | None = None + attempt_checks: list[str] = [] for attempt in range(1, LLM_BATCH_MAX_ATTEMPTS + 1): try: if format_source is not None: return self._format_batch(batch, format_source) return self._invoke_batch(batch, prompt) except _StructuredResponseValidationError as exc: + attempt_checks.append(exc.check) + exc.attempt_checks = tuple(attempt_checks) if format_source is not None: # Retry transport failures, never a returned formatting result. raise @@ -1324,12 +1387,15 @@ async def _ainvoke_batch_with_retries( structured_retries = 0 connection_retries = 0 format_source: str | None = None + attempt_checks: list[str] = [] for attempt in range(1, LLM_BATCH_MAX_ATTEMPTS + 1): try: if format_source is not None: return await self._aformat_batch(batch, format_source) return await self._ainvoke_batch(batch, prompt) except _StructuredResponseValidationError as exc: + attempt_checks.append(exc.check) + exc.attempt_checks = tuple(attempt_checks) if format_source is not None: # Retry transport failures, never a returned formatting result. raise @@ -1424,16 +1490,19 @@ def run_batches_detailed( result = self._invoke_batch_with_retries(batch, prompt, retry_outcome=retry_outcome) outcome.successful.append(result) except _StructuredResponseValidationError as exc: + diagnostic = exc.diagnostic(self.response_schema) logger.warning( - "LLM response validation failed for %s (%s)", + "LLM response validation failed for %s (%s; %s)", batch.file_label, exc.reason, + diagnostic, ) outcome.failures.append( BatchFailure( batch=batch, error_class=ValidationError.__name__, reason=exc.reason, + diagnostic=diagnostic, ) ) except LLMRuntimeLimitError as exc: @@ -1542,16 +1611,19 @@ async def _process( outcome = BatchExecutionResult() for batch, result, retry_outcome in zip(batches, results, retry_outcomes, strict=True): if isinstance(result, _StructuredResponseValidationError): + diagnostic = result.diagnostic(self.response_schema) logger.warning( - "LLM response validation failed for %s (%s)", + "LLM response validation failed for %s (%s; %s)", batch.file_label, result.reason, + diagnostic, ) outcome.failures.append( BatchFailure( batch=batch, error_class=ValidationError.__name__, reason=result.reason, + diagnostic=diagnostic, ) ) continue diff --git a/src/skillspector/llm_utils.py b/src/skillspector/llm_utils.py index b6d383101..b2f2351a2 100644 --- a/src/skillspector/llm_utils.py +++ b/src/skillspector/llm_utils.py @@ -50,7 +50,7 @@ from langchain_core.utils.function_calling import convert_to_openai_tool from langchain_openai.chat_models.base import BaseChatOpenAI from openai import ContentFilterFinishReasonError, LengthFinishReasonError, pydantic_function_tool -from pydantic import BaseModel +from pydantic import BaseModel, ValidationError from skillspector.inference_usage import ( InferenceUsageCollector, @@ -75,8 +75,10 @@ from skillspector.providers.openai import OpenAIProvider from skillspector.structured_response import ( MAX_RESPONSE_CHARS, + ResponseCheck, ResponseIssue, parse_response_object, + schema_error_paths, validate_message, validate_payload, ) @@ -213,7 +215,11 @@ def _extract_json_object(raw: str) -> dict: (fail-closed) when no JSON object can be parsed. """ if len(raw) > MAX_RESPONSE_CHARS: - raise StructuredOutputParseError("response exceeds size limit", issue=ResponseIssue.LIMIT) + raise StructuredOutputParseError( + "response exceeds size limit", + issue=ResponseIssue.LIMIT, + check=ResponseCheck.SIZE_LIMIT, + ) text = raw.strip() if text.startswith("```"): # Drop the opening fence line (``` or ```json) and any closing fence. @@ -234,7 +240,9 @@ def _extract_json_object(raw: str) -> dict: except StructuredOutputParseError as exc: if exc.issue != ResponseIssue.INVALID_JSON: raise - raise StructuredOutputParseError("could not extract a JSON object from CLI response") + raise StructuredOutputParseError( + "could not extract a JSON object from CLI response", check=ResponseCheck.CLI_JSON_MISSING + ) def _complete_agent_cli( @@ -470,7 +478,9 @@ def bind_structured_output( kwargs = structured_output_kwargs(model, provider, preferred_method=preferred_method) if preserve_text is not None and not isinstance(llm, BaseChatModel): raise StructuredOutputParseError( - "provider cannot verify lossless formatting", issue=ResponseIssue.FORMAT_CHANGED + "provider cannot verify lossless formatting", + issue=ResponseIssue.FORMAT_CHANGED, + check=ResponseCheck.FORMAT_UNSUPPORTED, ) unforced = _binds_unforced_tool_call(llm, kwargs.get("method")) include_raw = isinstance(llm, BaseChatModel) @@ -499,7 +509,9 @@ def bind_structured_output( def check(result: object) -> object: if not isinstance(result, dict) or result.get("raw") is None: raise StructuredOutputParseError( - "model returned no response", issue=ResponseIssue.EMPTY + "model returned no response", + issue=ResponseIssue.EMPTY, + check=ResponseCheck.NO_RESPONSE, ) return validate_message( result["raw"], @@ -595,6 +607,15 @@ def _ask(prompt: str) -> str: ) +def _schema_errors(error: BaseException, schema: type) -> tuple[str, ...]: + """Schema paths for a parser's Pydantic failure, or none for other parser errors.""" + cause = error if isinstance(error, ValidationError) else error.__cause__ + if isinstance(cause, ValidationError) and isinstance(schema, type): + if issubclass(schema, BaseModel): + return schema_error_paths(cause, schema) + return () + + def _validate_tool_call_result( structured: Runnable, schema: type, *, include_raw: bool = False ) -> Runnable: @@ -610,7 +631,10 @@ def _validate_tool_call_result( def _check(result: object) -> object: if include_raw: if not isinstance(result, dict): - raise StructuredOutputParseError(f"model returned invalid {tool} tool output") + raise StructuredOutputParseError( + f"model returned invalid {tool} tool output", + check=ResponseCheck.TOOL_OUTPUT_INVALID, + ) raw = result.get("raw") calls = getattr(raw, "tool_calls", None) if ( @@ -620,17 +644,21 @@ def _check(result: object) -> object: or getattr(raw, "invalid_tool_calls", None) ): raise StructuredOutputParseError( - f"model must return exactly one valid {tool} tool call" + f"model must return exactly one valid {tool} tool call", + check=ResponseCheck.TOOL_CALL_COUNT, ) error = result.get("parsing_error") if error is not None: raise StructuredOutputParseError( - f"model returned invalid {tool} tool arguments" + f"model returned invalid {tool} tool arguments", + check=ResponseCheck.TOOL_ARGUMENTS_INVALID, + schema_errors=_schema_errors(error, schema), ) from error result = result.get("parsed") if result is None: raise StructuredOutputParseError( - f"model answered in prose instead of calling the {tool} tool" + f"model answered in prose instead of calling the {tool} tool", + check=ResponseCheck.TOOL_CALL_MISSING, ) return result @@ -639,7 +667,9 @@ def _invoke(prompt: Any, config: RunnableConfig) -> object: return _check(structured.invoke(prompt, config=config)) except OutputParserException as exc: raise StructuredOutputParseError( - f"model returned invalid {tool} tool arguments" + f"model returned invalid {tool} tool arguments", + check=ResponseCheck.TOOL_ARGUMENTS_INVALID, + schema_errors=_schema_errors(exc, schema), ) from exc async def _ainvoke(prompt: Any, config: RunnableConfig) -> object: @@ -647,7 +677,9 @@ async def _ainvoke(prompt: Any, config: RunnableConfig) -> object: return _check(await structured.ainvoke(prompt, config=config)) except OutputParserException as exc: raise StructuredOutputParseError( - f"model returned invalid {tool} tool arguments" + f"model returned invalid {tool} tool arguments", + check=ResponseCheck.TOOL_ARGUMENTS_INVALID, + schema_errors=_schema_errors(exc, schema), ) from exc return RunnableLambda(_invoke, _ainvoke) diff --git a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py index 34384a81d..74f7e6e6d 100644 --- a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py +++ b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py @@ -1581,6 +1581,7 @@ def iter_candidates() -> Iterator[_TP4Candidate]: end_line=failure.batch.end_line, reason=failure.reason, error_class=failure.error_class, + diagnostic=failure.diagnostic, observed_seconds=( 0.0 if failure.reason is LedgerReason.RUNTIME_LIMIT else None ), diff --git a/src/skillspector/nodes/meta_analyzer.py b/src/skillspector/nodes/meta_analyzer.py index 1f34d2f1f..17f66680f 100644 --- a/src/skillspector/nodes/meta_analyzer.py +++ b/src/skillspector/nodes/meta_analyzer.py @@ -656,6 +656,7 @@ def _meta_ledger_response( input_finding_ids=input_ids, emitted_finding_ids=input_ids, error_class=failure.error_class, + diagnostic=failure.diagnostic, ) ) if not events: diff --git a/src/skillspector/structured_response.py b/src/skillspector/structured_response.py index bc0d3473d..c69133b35 100644 --- a/src/skillspector/structured_response.py +++ b/src/skillspector/structured_response.py @@ -15,7 +15,33 @@ MAX_RESPONSE_CHARS = 1_048_576 MAX_RESPONSE_DEPTH = 64 +MAX_SCHEMA_ERRORS = 5 _NUMBER = re.compile(r"-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?(?:[eE][+-]?[0-9]+)?") +_SCHEMA_ERROR_TYPE = re.compile(r"[a-z][a-z0-9_]{0,63}") +_CODE_FENCE = re.compile(r"```[A-Za-z0-9_+-]*") +# Completion labels that providers return verbatim; anything else is reported as "other". +_FINISH_LABELS = frozenset( + { + "stop", + "end_turn", + "tool_use", + "tool_calls", + "length", + "max_tokens", + "max_output_tokens", + "refusal", + "content_filter", + "safety", + "guardrail_intervened", + "pause_turn", + "completed", + "incomplete", + "in_progress", + "queued", + "failed", + "cancelled", + } +) class ResponseIssue(StrEnum): @@ -31,8 +57,57 @@ class ResponseIssue(StrEnum): FORMAT_CHANGED = "format_changed" +class ResponseCheck(StrEnum): + """The specific validation check that rejected an answer. + + ``ResponseIssue`` selects the ledger reason; this finer code tells which + bounded check failed without retaining any of the model's text. + """ + + SIZE_LIMIT = "size_limit" + NESTING_LIMIT = "nesting_limit" + NOT_JSON_OBJECT = "not_json_object" + UNBALANCED_CONTAINERS = "unbalanced_containers" + TRAILING_CONTENT = "trailing_content" + INVALID_STRING = "invalid_string" + NON_FINITE_NUMBER = "non_finite_number" + NON_JSON_TOKEN = "non_json_token" + MALFORMED_JSON = "malformed_json" + DUPLICATE_FIELD = "duplicate_field" + MISSING_FINDINGS = "missing_findings" + MISSING_FIELD = "missing_field" + INVALID_FIELD = "invalid_field" + EMPTY_RESPONSE = "empty_response" + NO_RESPONSE = "no_response" + NO_USABLE_ANSWER = "no_usable_answer" + NO_STRUCTURED_RESPONSE = "no_structured_response" + TOOL_CALL_COUNT = "tool_call_count" + TOOL_CALL_AMBIGUOUS = "tool_call_ambiguous" + TOOL_CALL_UNEXPECTED = "tool_call_unexpected" + TOOL_CALL_MISSING = "tool_call_missing" + TOOL_ARGUMENTS_INVALID = "tool_arguments_invalid" + TOOL_OUTPUT_INVALID = "tool_output_invalid" + CLI_JSON_MISSING = "cli_json_missing" + TRUNCATED = "truncated" + REFUSED = "refused" + INCOMPLETE = "incomplete" + FORMAT_CHANGED = "format_changed" + FORMAT_UNVERIFIED = "format_unverified" + FORMAT_UNSUPPORTED = "format_unsupported" + FORMAT_INPUT_LIMIT = "format_input_limit" + + +_SHAPED_CHECKS = frozenset( + {ResponseCheck.NOT_JSON_OBJECT, ResponseCheck.TRAILING_CONTENT, ResponseCheck.NON_JSON_TOKEN} +) + + class StructuredOutputParseError(ValueError): - """Content-free diagnostic; response text is retained only for bounded recovery.""" + """Content-free diagnostic; response text is retained only for bounded recovery. + + ``check``, ``channel``, ``finish``, ``shape`` and ``schema_errors`` are fixed + codes or schema-declared field paths, so they can be reported without the answer. + """ def __init__( self, @@ -40,10 +115,20 @@ def __init__( *, issue: ResponseIssue = ResponseIssue.INVALID_JSON, repair_text: str | None = None, + check: ResponseCheck | None = None, + schema_errors: tuple[str, ...] = (), ) -> None: super().__init__(message) self.issue = issue self.repair_text = repair_text + self.check = check + self.schema_errors = schema_errors + # Set by validate_message: whether the answer arrived as a tool call or + # as text, and the provider's allowlisted completion label. + self.channel: str | None = None + self.finish: str | None = None + # Set when text was not a bare JSON object: where prose or fences sat. + self.shape: str | None = None @property def retryable(self) -> bool: @@ -58,7 +143,11 @@ def retryable(self) -> bool: def _json_body(text: str) -> str: if len(text) > MAX_RESPONSE_CHARS: - raise StructuredOutputParseError("response exceeds size limit", issue=ResponseIssue.LIMIT) + raise StructuredOutputParseError( + "response exceeds size limit", + issue=ResponseIssue.LIMIT, + check=ResponseCheck.SIZE_LIMIT, + ) text = text.strip() lines = text.splitlines() if len(lines) >= 3 and lines[0] in ("```", "```json") and lines[-1] == "```": @@ -74,7 +163,9 @@ def formatting_tokens(text: str) -> tuple[str, ...]: """ text = _json_body(text) if not text.startswith("{") or not text.endswith("}"): - raise StructuredOutputParseError("response is not a complete JSON object") + raise StructuredOutputParseError( + "response is not a complete JSON object", check=ResponseCheck.NOT_JSON_OBJECT + ) decoder = json.JSONDecoder() tokens: list[str] = [] stack: list[str] = [] @@ -88,22 +179,31 @@ def formatting_tokens(text: str) -> tuple[str, ...]: stack.append("}" if char == "{" else "]") if len(stack) > MAX_RESPONSE_DEPTH: raise StructuredOutputParseError( - "response exceeds nesting limit", issue=ResponseIssue.LIMIT + "response exceeds nesting limit", + issue=ResponseIssue.LIMIT, + check=ResponseCheck.NESTING_LIMIT, ) tokens.append(char) offset += 1 elif char in "}]": if not stack or stack.pop() != char: - raise StructuredOutputParseError("response has incomplete containers") + raise StructuredOutputParseError( + "response has incomplete containers", + check=ResponseCheck.UNBALANCED_CONTAINERS, + ) tokens.append(char) offset += 1 if not stack and offset != len(text): - raise StructuredOutputParseError("response contains multiple objects") + raise StructuredOutputParseError( + "response contains multiple objects", check=ResponseCheck.TRAILING_CONTENT + ) elif char == '"': try: value, end = decoder.raw_decode(text, offset) except json.JSONDecodeError as exc: - raise StructuredOutputParseError("response has an incomplete string") from exc + raise StructuredOutputParseError( + "response has an incomplete string", check=ResponseCheck.INVALID_STRING + ) from exc tokens.append(json.dumps(value, ensure_ascii=True)) offset = end else: @@ -114,16 +214,23 @@ def formatting_tokens(text: str) -> tuple[str, ...]: if number: token = number.group() if not math.isfinite(float(token)): - raise StructuredOutputParseError("response has a non-finite number") + raise StructuredOutputParseError( + "response has a non-finite number", + check=ResponseCheck.NON_FINITE_NUMBER, + ) tokens.append(token) offset = number.end() elif literal: tokens.append(literal) offset += len(literal) else: - raise StructuredOutputParseError("response contains non-JSON text") + raise StructuredOutputParseError( + "response contains non-JSON text", check=ResponseCheck.NON_JSON_TOKEN + ) if stack: - raise StructuredOutputParseError("response has incomplete containers") + raise StructuredOutputParseError( + "response has incomplete containers", check=ResponseCheck.UNBALANCED_CONTAINERS + ) return tuple(tokens) @@ -132,18 +239,90 @@ def _unique_fields(pairs: list[tuple[str, Any]]) -> dict[str, Any]: for key, value in pairs: if key in result: raise StructuredOutputParseError( - "response contains duplicate fields", issue=ResponseIssue.INVALID_SCHEMA + "response contains duplicate fields", + issue=ResponseIssue.INVALID_SCHEMA, + check=ResponseCheck.DUPLICATE_FIELD, ) result[key] = value return result +def _declared_field_names(schema: type[BaseModel]) -> frozenset[str]: + """Property names anywhere in *schema*'s JSON schema, including nested models.""" + names: set[str] = set() + pending: list[object] = [schema.model_json_schema()] + while pending: + node = pending.pop() + if isinstance(node, dict): + properties = node.get("properties") + if isinstance(properties, dict): + names.update(key for key in properties if isinstance(key, str)) + pending.extend(node.values()) + elif isinstance(node, list): + pending.extend(node) + return frozenset(names) + + +def schema_error_paths(exc: ValidationError, schema: type[BaseModel]) -> tuple[str, ...]: + """Return ``field.path:error_type`` pairs that never echo model-supplied text. + + Path parts are list indexes or names the schema declares; any other key, + such as an unexpected field the model invented, is reported as ``*``. + """ + try: + names = _declared_field_names(schema) + paths: list[str] = [] + for error in exc.errors(include_url=False, include_context=False, include_input=False): + parts = [ + str(part) + if isinstance(part, int) and not isinstance(part, bool) + else part + if isinstance(part, str) and part in names + else "*" + for part in error.get("loc", ()) + ] + error_type = error.get("type") + if not isinstance(error_type, str) or not _SCHEMA_ERROR_TYPE.fullmatch(error_type): + error_type = "other" + path = f"{'.'.join(parts) or '$'}:{error_type}" + if path not in paths: + paths.append(path) + if len(paths) == MAX_SCHEMA_ERRORS: + break + return tuple(paths) + except Exception: + # A diagnostic must never replace the validation failure it describes. + return () + + +def _text_shape(text: str) -> str: + """Fixed labels for prose or fences around a JSON-looking object, never the text.""" + start = text.find("{") + end = text.rfind("}") + if start < 0 or end < start: + return "no_object" + before = text[:start] + after = text[end + 1 :] + labels = [ + label + for label, present in ( + ("prefix", bool(_CODE_FENCE.sub("", before).strip())), + ("fence", "```" in before or "```" in after), + ("suffix", bool(_CODE_FENCE.sub("", after).strip())), + ) + if present + ] + return "+".join(labels) or "object" + + def validate_payload(payload: dict, schema: type[BaseModel]) -> object: """Require an explicit findings list even when the internal model has a default.""" fields = getattr(schema, "model_fields", {}) if "findings" in fields and "findings" not in payload: raise StructuredOutputParseError( - "response omits findings", issue=ResponseIssue.MISSING_FIELDS + "response omits findings", + issue=ResponseIssue.MISSING_FIELDS, + check=ResponseCheck.MISSING_FINDINGS, ) try: return schema.model_validate(payload, extra="forbid") @@ -154,6 +333,8 @@ def validate_payload(payload: dict, schema: type[BaseModel]) -> object: if missing else "response does not match the required fields", issue=ResponseIssue.MISSING_FIELDS if missing else ResponseIssue.INVALID_SCHEMA, + check=ResponseCheck.MISSING_FIELD if missing else ResponseCheck.INVALID_FIELD, + schema_errors=schema_error_paths(exc, schema), ) from exc @@ -164,7 +345,9 @@ def parse_response_object(text: str) -> dict: try: payload = json.loads(body, object_pairs_hook=_unique_fields) except json.JSONDecodeError as exc: - raise StructuredOutputParseError("model returned malformed JSON") from exc + raise StructuredOutputParseError( + "model returned malformed JSON", check=ResponseCheck.MALFORMED_JSON + ) from exc assert isinstance(payload, dict) return payload @@ -180,20 +363,30 @@ def validate_response_text( body = _json_body(text) if not body: raise StructuredOutputParseError( - "model returned an empty response", issue=ResponseIssue.EMPTY + "model returned an empty response", + issue=ResponseIssue.EMPTY, + check=ResponseCheck.EMPTY_RESPONSE, ) # Bound nesting before invoking the recursive JSON decoder. - tokens = formatting_tokens(body) + try: + tokens = formatting_tokens(body) + except StructuredOutputParseError as exc: + if exc.check in _SHAPED_CHECKS: + exc.shape = _text_shape(body) + raise try: payload = json.loads(body, object_pairs_hook=_unique_fields) except json.JSONDecodeError as exc: raise StructuredOutputParseError( "model returned malformed JSON", repair_text=body if completed and preserve_text is None else None, + check=ResponseCheck.MALFORMED_JSON, ) from exc if preserve_text is not None and tokens != formatting_tokens(preserve_text): raise StructuredOutputParseError( - "formatting changed assessment content", issue=ResponseIssue.FORMAT_CHANGED + "formatting changed assessment content", + issue=ResponseIssue.FORMAT_CHANGED, + check=ResponseCheck.FORMAT_CHANGED, ) if allow_json_wrapper and len(payload) == 1: # Exact gateway argument envelopes observed on completed live responses. @@ -204,6 +397,28 @@ def validate_response_text( return validate_payload(payload, schema) +def _finish_label(message: object) -> str: + metadata = getattr(message, "response_metadata", {}) or {} + stop = ( + metadata.get("finish_reason") or metadata.get("stop_reason") or metadata.get("stopReason") + ) + if not isinstance(stop, str): + stop = metadata.get("status") + if not isinstance(stop, str): + return "none" + stop = stop.lower() + return stop if stop in _FINISH_LABELS else "other" + + +def _answer_channel(message: object) -> str: + if getattr(message, "tool_calls", None) or getattr(message, "invalid_tool_calls", None): + return "tool_call" + content = getattr(message, "content", "") + if isinstance(content, str): + return "text" if content.strip() else "empty" + return "content_blocks" if isinstance(content, list) else "none" + + def validate_message( message: object, schema: type[BaseModel], @@ -214,6 +429,34 @@ def validate_message( preserve_text: str | None = None, ) -> object: """Read stop/refusal evidence before parsing either text or tool arguments.""" + try: + return _validate_message( + message, + schema, + tool=tool, + require_tool=require_tool, + allow_json_wrapper=allow_json_wrapper, + preserve_text=preserve_text, + ) + except StructuredOutputParseError as exc: + try: + exc.channel = _answer_channel(message) + exc.finish = _finish_label(message) + except Exception: + # A diagnostic must never replace the validation failure it describes. + pass + raise + + +def _validate_message( + message: object, + schema: type[BaseModel], + *, + tool: str, + require_tool: bool, + allow_json_wrapper: bool, + preserve_text: str | None, +) -> object: metadata = getattr(message, "response_metadata", {}) or {} additional = getattr(message, "additional_kwargs", {}) or {} content = getattr(message, "content", "") @@ -227,7 +470,9 @@ def validate_message( stop = incomplete.get("reason") if stop in {"length", "max_tokens", "max_output_tokens"}: raise StructuredOutputParseError( - "model answer was cut short", issue=ResponseIssue.TRUNCATED + "model answer was cut short", + issue=ResponseIssue.TRUNCATED, + check=ResponseCheck.TRUNCATED, ) if ( additional.get("refusal") @@ -236,19 +481,25 @@ def validate_message( and any(isinstance(block, dict) and block.get("type") == "refusal" for block in content) ): raise StructuredOutputParseError( - "model declined the assessment", issue=ResponseIssue.REFUSED + "model declined the assessment", + issue=ResponseIssue.REFUSED, + check=ResponseCheck.REFUSED, ) if ( status in {"incomplete", "in_progress", "queued", "failed", "cancelled"} or stop == "pause_turn" ): raise StructuredOutputParseError( - "model did not finish the assessment", issue=ResponseIssue.INCOMPLETE + "model did not finish the assessment", + issue=ResponseIssue.INCOMPLETE, + check=ResponseCheck.INCOMPLETE, ) completed = stop in {"stop", "end_turn", "tool_use", "tool_calls"} or status == "completed" if preserve_text is not None and not completed: raise StructuredOutputParseError( - "formatting response has no completion evidence", issue=ResponseIssue.FORMAT_CHANGED + "formatting response has no completion evidence", + issue=ResponseIssue.FORMAT_CHANGED, + check=ResponseCheck.FORMAT_UNVERIFIED, ) calls = list(getattr(message, "tool_calls", []) or []) + list( getattr(message, "invalid_tool_calls", []) or [] @@ -256,7 +507,9 @@ def validate_message( if require_tool or calls: if len(calls) != 1 or calls[0].get("name") != tool: raise StructuredOutputParseError( - f"model must return exactly one {tool} tool call", issue=ResponseIssue.INVALID_TOOL + f"model must return exactly one {tool} tool call", + issue=ResponseIssue.INVALID_TOOL, + check=ResponseCheck.TOOL_CALL_COUNT, ) args = calls[0].get("args") wire_calls = additional.get("tool_calls") @@ -271,12 +524,16 @@ def validate_message( if wire_calls is not None: if not isinstance(wire_calls, list) or len(wire_calls) != 1: raise StructuredOutputParseError( - "model returned ambiguous tool calls", issue=ResponseIssue.INVALID_TOOL + "model returned ambiguous tool calls", + issue=ResponseIssue.INVALID_TOOL, + check=ResponseCheck.TOOL_CALL_AMBIGUOUS, ) function = wire_calls[0].get("function") if isinstance(wire_calls[0], dict) else None if not isinstance(function, dict) or function.get("name") != tool: raise StructuredOutputParseError( - "model returned an unexpected tool", issue=ResponseIssue.INVALID_TOOL + "model returned an unexpected tool", + issue=ResponseIssue.INVALID_TOOL, + check=ResponseCheck.TOOL_CALL_UNEXPECTED, ) args = function.get("arguments") text = args if isinstance(args, str) else json.dumps(args, ensure_ascii=True) @@ -292,7 +549,9 @@ def validate_message( ) else: raise StructuredOutputParseError( - "model returned no usable answer", issue=ResponseIssue.EMPTY + "model returned no usable answer", + issue=ResponseIssue.EMPTY, + check=ResponseCheck.NO_USABLE_ANSWER, ) return validate_response_text( text, diff --git a/tests/test_mcp_tool_poisoning.py b/tests/test_mcp_tool_poisoning.py index f70f9a3f6..3b3baefc4 100644 --- a/tests/test_mcp_tool_poisoning.py +++ b/tests/test_mcp_tool_poisoning.py @@ -1802,10 +1802,15 @@ def test_wrapped_json_schema_response_stays_incomplete( assert structured_llm.calls == 4 assert result["analyzer_status_events"][0]["status"] == "degraded" - assert any( - event.get("reason_code") is LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID + assert [ + event.get("diagnostic") for event in result["inspection_ledger"] - ) + if event.get("reason_code") is LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID + ] == [ + # The wrapper key the model invented is never echoed; the schema path is. + "check=missing_field; schema=is_mismatch:missing; attempts=4; " + "checks=missing_field,missing_field,missing_field,missing_field" + ] def test_missing_structured_result_stays_incomplete( self, monkeypatch: pytest.MonkeyPatch diff --git a/tests/unit/test_structured_response.py b/tests/unit/test_structured_response.py index 3e3e55247..eecb597a4 100644 --- a/tests/unit/test_structured_response.py +++ b/tests/unit/test_structured_response.py @@ -14,8 +14,18 @@ from langchain_core.messages import AIMessage from langchain_openai import ChatOpenAI -from skillspector.inspection_ledger import LedgerReason -from skillspector.llm_analyzer_base import Batch, LLMAnalysisResult, LLMAnalyzerBase +from skillspector.inspection_ledger import ( + LedgerOutcome, + LedgerReason, + finalize_ledger, + ledger_event, +) +from skillspector.llm_analyzer_base import ( + Batch, + LLMAnalysisResult, + LLMAnalyzerBase, + ledger_events_for_batches, +) from skillspector.llm_utils import bind_structured_output, register_chat_model_provider from skillspector.nodes.analyzers.mcp_tool_poisoning import _TP4AnalysisResult, _TP4Analyzer from skillspector.nodes.analyzers.semantic_security_discovery import node as security_discovery @@ -24,10 +34,12 @@ from skillspector.providers.openai import OpenAIProvider from skillspector.structured_response import ( MAX_RESPONSE_DEPTH, + ResponseCheck, ResponseIssue, StructuredOutputParseError, formatting_tokens, validate_message, + validate_payload, validate_response_text, ) @@ -752,3 +764,131 @@ def respond(request): assert error.value.issue == ResponseIssue.INVALID_SCHEMA else: assert runnable.invoke("Assess").findings == [] + + +# Diagnostics name the failed check, never the model's text. A response carries +# this canary so each test can prove it never reaches a diagnostic or log line. +_CANARY = "canary-7f3a9c" + + +@pytest.mark.parametrize( + "text, check, shape", + [ + ('Here is the result:\n```json\n{"findings": []}\n```', "not_json_object", "prefix+fence"), + ('{"findings": []}\nNo issues found.', "not_json_object", "suffix"), + ('{"findings": []} {"findings": []}', "trailing_content", "object"), + ("No issues found.", "not_json_object", "no_object"), + ('{"findings": [}]}', "unbalanced_containers", None), + ('{"findings": ["\\$"]}', "invalid_string", None), + ('{"findings": [],}', "malformed_json", None), + ("", "empty_response", None), + ], +) +def test_text_failures_name_the_rejecting_check(text, check, shape): + message = AIMessage(content=text, response_metadata={"finish_reason": "stop"}) + with pytest.raises(StructuredOutputParseError) as error: + validate_message(message, LLMAnalysisResult, tool="LLMAnalysisResult") + assert error.value.check == check + assert error.value.shape == shape + assert error.value.channel == ("text" if text else "empty") + assert error.value.finish == "stop" + + +def test_schema_errors_report_declared_paths_without_values_or_invented_keys(): + finding = { + "rule_id": "SDI-1", + "message": _CANARY, + "severity": _CANARY, + "start_line": 3, + _CANARY: "x", + } + with pytest.raises(StructuredOutputParseError) as error: + validate_payload({"findings": [finding]}, LLMAnalysisResult) + assert error.value.check == ResponseCheck.INVALID_FIELD + assert error.value.schema_errors == ( + "findings.0.severity:literal_error", + "findings.0.*:extra_forbidden", + ) + + +def test_unrecognized_completion_labels_are_not_echoed(): + message = AIMessage(content="prose", response_metadata={"finish_reason": _CANARY}) + with pytest.raises(StructuredOutputParseError) as error: + validate_message(message, LLMAnalysisResult, tool="LLMAnalysisResult") + assert error.value.finish == "other" + + +@pytest.mark.parametrize("method", ["json_schema", "function_calling"]) +@pytest.mark.parametrize("asynchronous", [False, True]) +def test_exhausted_retries_keep_each_attempts_check_in_the_ledger( + http_model, caplog, method, asynchronous +): + answer = f'Assessment of {_CANARY}:\n```json\n{{"findings": []}}\n```' + requests = http_model([(answer, "stop")], method=method) + analyzer = make_analyzer("semantic_developer_intent") + batches = [Batch(file_path="a.py", content="x")] + outcome = ( + asyncio.run(analyzer.arun_batches_detailed(batches)) + if asynchronous + else analyzer.run_batches_detailed(batches) + ) + + channel = "text" if method == "json_schema" else "tool_call" + finish = "stop" if method == "json_schema" else "tool_calls" + expected = ( + f"check=not_json_object; channel={channel}; finish={finish}; shape=prefix+fence; " + "attempts=4; checks=not_json_object,not_json_object,not_json_object,not_json_object" + ) + assert len(requests) == 4 + assert not outcome.successful + failure = outcome.failures[0] + assert failure.reason == LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID + assert failure.error_class == "ValidationError" + assert failure.diagnostic == expected + events, status = ledger_events_for_batches("semantic_developer_intent", outcome) + assert events[0]["diagnostic"] == expected + completeness, _ = finalize_ledger( + { + "components": ["a.py"], + "findings": [], + "inspection_ledger": events, + "analyzer_status_events": [status], + } + ) + assert [row.get("diagnostic") for row in completeness["ledger_exceptions"]] == [expected] + assert _CANARY not in caplog.text + assert expected in caplog.text + + +def test_format_recovery_failure_keeps_the_original_attempt_checks(http_model): + source = '{"findings":[{"rule_id":"SDI-1","message":"m","severity":"HIGH","start_line":1}],}' + http_model([(source, "stop"), ('{"findings": []}', "stop")]) + outcome = make_analyzer("discovery").run_batches_detailed( + [Batch(file_path="a.py", content="x")] + ) + assert outcome.failures[0].reason == LedgerReason.LLM_FORMAT_CHANGED + assert outcome.failures[0].diagnostic == ( + "check=format_changed; channel=text; finish=stop; " + "attempts=2; checks=malformed_json,format_changed" + ) + + +@pytest.mark.parametrize( + "diagnostic", + [ + f"check=not_json_object; note={_CANARY} text", + 'check=invalid_field; schema=findings.0."severity":literal_error', + "Check=not_json_object", + "check=" + "a" * 600, + ], +) +def test_ledger_drops_diagnostics_outside_the_fixed_code_format(diagnostic): + event = ledger_event( + outcome=LedgerOutcome.SKIPPED, + phase="semantic", + path="a.py", + analyzer_id="semantic_developer_intent", + reason=LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID, + diagnostic=diagnostic, + ) + assert "diagnostic" not in event From 8ed0fd7ff2830ab41eeff8f5062997661475bf7b Mon Sep 17 00:00:00 2001 From: Mohit Gupta Date: Wed, 4 Nov 2026 11:08:00 +0530 Subject: [PATCH 2/4] fix(sc4): retain up to 64 OSV advisories per locked package A widely used package can carry more than sixteen advisories for one locked version (filelock 4.1.1 has 17), and truncating them left the dependency manifest partially inspected with output_limit. Raise the per-package cap to 64; the per-scan detail-request and result budgets still bound latency, and exceeding any cap still reports output_limit. Signed-off-by: Mohit Gupta Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + docs/SC4-osv-live-vulnerability-lookups.md | 2 +- .../nodes/analyzers/osv_client.py | 5 ++++- tests/unit/test_osv_client.py | 20 +++++++++++++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da419aa16..e7194dbc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Fixes * Backport scoped `allowed-tools` grant handling from #768: recognize declarations such as `Bash(git:*)`, preserve spaces and commas inside scopes, and keep malformed grants or embedded tool names from declaring shell capability. Command restrictions remain intact in the manifest. * Report which check rejected a model answer. Ledger rows and logs for rejected structured responses carry a `diagnostic` built only from fixed codes (check, channel, completion label, JSON shape, schema paths and per-attempt checks), never model output. +* Retain up to 64 OSV advisories per locked package (was 16), so a dependency such as `filelock` with 17 advisories no longer leaves `poetry.lock` partial with `output_limit`. Per-scan detail and result budgets are unchanged. --- ### 2.13.3 (Unreleased) ### Security and integrity fixes diff --git a/docs/SC4-osv-live-vulnerability-lookups.md b/docs/SC4-osv-live-vulnerability-lookups.md index c38778688..b05ac72e5 100644 --- a/docs/SC4-osv-live-vulnerability-lookups.md +++ b/docs/SC4-osv-live-vulnerability-lookups.md @@ -103,7 +103,7 @@ _analyze_dependencies(content, file_path) | **Sync vs. async** | Synchronous `httpx.Client` | The analyzer pipeline is sync. A single batch call completes in <500 ms for typical dependency files. | | **Caching** | In-memory dict with 1-hour TTL, keyed on `(name, version, ecosystem)` | Prevents redundant API calls when multiple skills share dependencies. Skillspector runs are short-lived CLI invocations, so memory is not a concern. | | **Graceful degradation** | On timeout/network error, fall back to the static list (`_FALLBACK_VULNERABLE_*`) | Ensures the tool works in air-gapped or offline environments. A warning is logged when falling back. | -| **Finding detail level** | Batch query returns vuln IDs; detail fetched via `GET /v1/vulns/{id}` for up to 10 vulns per package | Keeps latency low — the 10-vulnerability cap limits the number of sequential detail fetches per package. The implementation selects the first 10 vulnerability IDs as returned by the OSV batch API (`_fetch_vuln_details(vuln_ids[:10])`) with no severity-based sorting or prioritisation. When more than 10 IDs are returned, the OSV client logs a warning indicating the total count and that only the first 10 will be processed, so users are alerted to the truncation. Finding messages include OSV/GHSA/CVE IDs with summaries. | +| **Finding detail level** | Batch query returns vuln IDs; detail fetched via `GET /v1/vulns/{id}` for up to 64 vulns per package (`MAX_OSV_VULNS_PER_PACKAGE`) | The per-package cap and the aggregate per-scan budgets (64 detail requests, 256 retained results) bound latency. The implementation selects vulnerability IDs in the order the OSV batch API returns them, with no severity-based sorting or prioritisation. When a package or the scan exceeds a cap, the dependency manifest is reported as partially inspected with `output_limit`, because an omitted advisory could carry the worst severity. Finding messages include OSV/GHSA/CVE IDs with summaries. | | **Confidence mapping** | Map OSV severity → confidence: CRITICAL=0.9, HIGH=0.8, MEDIUM=0.7, LOW=0.6 | Replaces the per-entry hardcoded confidence values with a systematic mapping. | | **Timeout** | 10 s connect + read | Generous for a single POST. If exceeded, fallback activates gracefully. | | **Severity aggregation** | When multiple advisories affect one package, the worst severity is used for the finding | A single SC4 finding is emitted per package with a count and summary of all advisories. | diff --git a/src/skillspector/nodes/analyzers/osv_client.py b/src/skillspector/nodes/analyzers/osv_client.py index 7fc577c17..70002d876 100644 --- a/src/skillspector/nodes/analyzers/osv_client.py +++ b/src/skillspector/nodes/analyzers/osv_client.py @@ -61,7 +61,10 @@ MAX_OSV_DETAIL_REQUESTS = 64 MAX_OSV_RESPONSE_BYTES = 4 * 1024 * 1024 MAX_OSV_RESULTS = 256 -MAX_OSV_VULNS_PER_PACKAGE = 16 +# Per-package advisories retained for one locked version. Widely used packages can +# carry more than a dozen advisories for one release, and truncating them makes +# the manifest partial; the aggregate detail and result budgets above still apply. +MAX_OSV_VULNS_PER_PACKAGE = 64 MAX_OSV_LIMITATIONS = 16 MAX_OSV_CACHE_ENTRIES = 4_096 MAX_OSV_PACKAGE_NAME_CHARS = 256 diff --git a/tests/unit/test_osv_client.py b/tests/unit/test_osv_client.py index 74c304a85..1ba745fbe 100644 --- a/tests/unit/test_osv_client.py +++ b/tests/unit/test_osv_client.py @@ -482,6 +482,26 @@ def test_retained_vulnerability_results_are_capped( assert [item.vuln_id for item in results[0]] == ["OSV-1"] assert any(item.reason is LedgerReason.OUTPUT_LIMIT for item in results.limitations) + def test_package_with_more_than_sixteen_advisories_keeps_complete_coverage(self) -> None: + """A locked package with 17 advisories (seen in a real poetry.lock) is not truncated.""" + vulns = [{"id": f"OSV-{index}"} for index in range(17)] + client = self._client_for_batches([{"results": [{"vulns": vulns}]}]) + + def detail(url: str, **_kwargs: object) -> MagicMock: + response = MagicMock() + response.raise_for_status = MagicMock() + response.json.return_value = {"id": url.rsplit("/", 1)[-1], "summary": "advisory"} + return response + + client.get.side_effect = detail + + with patch("skillspector.nodes.analyzers.osv_client.httpx.Client", return_value=client): + results = query_batch([("demo", "1.0.0")], ECOSYSTEM_PYPI) + + assert [item.vuln_id for item in results[0]] == [f"OSV-{index}" for index in range(17)] + assert client.get.call_count == 17 + assert not results.limitations + def test_truncated_provider_result_is_not_cached(self, monkeypatch: pytest.MonkeyPatch) -> None: import skillspector.nodes.analyzers.osv_client as osv From 6e33f7fa52d46694ad65deb3cf0364f5d2afacbd Mon Sep 17 00:00:00 2001 From: Mohit Gupta Date: Wed, 4 Nov 2026 11:08:00 +0530 Subject: [PATCH 3/4] fix(llm): ask gateway Claude routes for exactly one JSON object The OpenAI provider binds a strict json_schema response format, and the analyzer prompts carry no output-format instruction. An OpenAI-compatible gateway can serve that format for Claude through a tool the model is not forced to call (Claude 5.5 rejects forced tool calls), so a prose answer leaves no JSON object and is rejected as llm_structured_response_invalid after the bounded retries. For Claude model IDs on OpenAI-protocol clients bound with json_schema, append an instruction to return exactly one JSON object matching the schema, without prose or code fences. Other models and methods keep their prompts; validation is unchanged. Signed-off-by: Mohit Gupta Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/skillspector/llm_utils.py | 23 +++++++++++++++ tests/unit/test_llm_utils.py | 53 +++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7194dbc0..e38c0ec9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ### Unreleased maintenance fixes ### Fixes * Backport scoped `allowed-tools` grant handling from #768: recognize declarations such as `Bash(git:*)`, preserve spaces and commas inside scopes, and keep malformed grants or embedded tool names from declaring shell capability. Command restrictions remain intact in the manifest. +* Ask Claude models reached through OpenAI-compatible gateways for exactly one JSON object when the schema is bound as `json_schema`. Gateways can serve that format through a tool Claude may skip, and a prose answer is rejected as `llm_structured_response_invalid`. Validation is unchanged. * Report which check rejected a model answer. Ledger rows and logs for rejected structured responses carry a `diagnostic` built only from fixed codes (check, channel, completion label, JSON shape, schema paths and per-attempt checks), never model output. * Retain up to 64 OSV advisories per locked package (was 16), so a dependency such as `filelock` with 17 advisories no longer leaves `poetry.lock` partial with `output_limit`. Per-scan detail and result budgets are unchanged. --- diff --git a/src/skillspector/llm_utils.py b/src/skillspector/llm_utils.py index b2f2351a2..624a42980 100644 --- a/src/skillspector/llm_utils.py +++ b/src/skillspector/llm_utils.py @@ -554,6 +554,13 @@ async def ainvoke(prompt: Any, config: RunnableConfig) -> object: ) | checked ) + if _gateway_claude_json_schema(llm, model, kwargs.get("method")): + return ( + RunnableLambda( + lambda prompt: f"{prompt}\n\n{_JSON_OBJECT_INSTRUCTION.format(tool=tool)}" + ) + | checked + ) return checked if unforced: return _require_tool_call(structured, schema, include_raw=include_raw) @@ -586,6 +593,22 @@ def _binds_unforced_tool_call(llm: object, method: str | None) -> bool: "Report your result by calling the {tool} tool exactly once. Do not answer in prose." ) +# OpenAI-compatible gateways can serve Claude's ``json_schema`` answer through a tool the +# model is not forced to call, so a prose answer leaves no JSON object to validate. +_JSON_OBJECT_INSTRUCTION = ( + "Return your result as exactly one JSON object that matches the required {tool} " + "schema. Do not add prose or Markdown code fences before or after it." +) + + +def _gateway_claude_json_schema(llm: object, model: str, method: str | None) -> bool: + """``True`` when an OpenAI-protocol client asks a Claude *model* for ``json_schema``.""" + return ( + isinstance(llm, BaseChatOpenAI) + and method in (None, "json_schema") + and "claude-" in model.rpartition("/")[2].lower() + ) + def _require_tool_call( structured: Runnable, schema: type, *, include_raw: bool = False diff --git a/tests/unit/test_llm_utils.py b/tests/unit/test_llm_utils.py index 5d2cc1a65..4be8cfe4e 100644 --- a/tests/unit/test_llm_utils.py +++ b/tests/unit/test_llm_utils.py @@ -1031,6 +1031,59 @@ def test_chat_openai_with_tool_choice_disabled_keeps_a_non_tool_method( assert request["response_format"]["json_schema"]["strict"] is True assert "tools" not in request + @pytest.mark.parametrize( + "model", + ["aws/anthropic/bedrock-claude-sonnet-5-5", "aws/anthropic/bedrock-claude-opus-5-5"], + ) + def test_gateway_claude_json_schema_asks_for_one_json_object( + self, monkeypatch: pytest.MonkeyPatch, model: str + ) -> None: + """A gateway can serve ``json_schema`` through a tool Claude may skip, so ask for the object.""" + monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + llm, requests = _stub_chat_openai( + monkeypatch, + [ + AIMessage( + content="The file looks fine.", response_metadata={"finish_reason": "stop"} + ), + AIMessage( + content='{"summary": "ok"}', + additional_kwargs={"parsed": Verdict(summary="ok")}, + response_metadata={"finish_reason": "stop"}, + ), + ], + ) + chain = bind_structured_output(llm, Verdict, model, provider=_PlainProvider()) + with pytest.raises(StructuredOutputParseError): + chain.invoke("analyse this") # type: ignore[attr-defined] + assert chain.invoke("analyse this") == Verdict(summary="ok") # type: ignore[attr-defined] + for prompt, request in requests: + assert prompt.startswith("analyse this\n\n") + assert "exactly one JSON object that matches the required Verdict schema" in prompt + assert request["response_format"]["type"] == "json_schema" + assert request["response_format"]["json_schema"]["strict"] is True + assert "tools" not in request + + @pytest.mark.parametrize( + ("model", "method", "expected"), + [ + ("aws/anthropic/bedrock-claude-sonnet-5-5", None, True), + ("aws/anthropic/bedrock-claude-sonnet-5-5", "json_schema", True), + ("azure/anthropic/claude-opus-5", None, True), + ("aws/anthropic/bedrock-claude-sonnet-5-5", "function_calling", False), + ("gpt-5.4", None, False), + ("anthropic/claude-x/gpt-5.4", None, False), + ], + ) + def test_json_object_instruction_is_limited_to_gateway_claude_json_schema( + self, monkeypatch: pytest.MonkeyPatch, model: str, method: str | None, expected: bool + ) -> None: + from skillspector.llm_utils import _gateway_claude_json_schema + + llm, _ = _stub_chat_openai(monkeypatch, []) + assert _gateway_claude_json_schema(llm, model, method) is expected + assert _gateway_claude_json_schema(_RecordingLLM(), model, method) is False + def test_cli_adapter_accepts_the_method_keyword(self) -> None: from skillspector.llm_utils import AgentCLIChatModel From 09c81a42b285ee9c19711c408ad2eb637ec89fd1 Mon Sep 17 00:00:00 2001 From: Mohit Gupta Date: Wed, 4 Nov 2026 11:08:00 +0530 Subject: [PATCH 4/4] fix(parser): bound backtick words in discarded Python strings The 2.13.3 shell-ownership backport (cd0e5791) stopped treating Python quotes as shell quotes, so a backtick word inside a string, such as "else `tool` on PATH" in a docstring, became its own command-word candidate. Its operands are bounded by the string's value, but the 2.13 string-value guard keeps the whole-source fallback whenever the module has any recognized shell path or an unlisted os/dunder use, so ordinary helpers that were complete in 2.12 and on main became static_parse_limit. - A docstring or other bare string statement is evaluated and dropped, so its own body bounds such a word whatever else the module does, unless the module reads docstrings back (__doc__, getdoc). - The guard treats os calls that run no program (makedirs, kill, readlink, ...) and name attributes (__name__, __qualname__, __module__, __file__) as inert. Shell sinks, reflection, dynamic imports and object escapes still keep the fallback, and a destructive operand inside a docstring still exhausts the bounded check. Signed-off-by: Mohit Gupta Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../analyzers/static_patterns_tool_misuse.py | 18 +- src/skillspector/python_tokens.py | 112 ++++++++++++- .../test_python_discarded_string_ownership.py | 156 ++++++++++++++++++ 4 files changed, 285 insertions(+), 2 deletions(-) create mode 100644 tests/nodes/analyzers/test_python_discarded_string_ownership.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e38c0ec9e..8a45d84aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ### Unreleased maintenance fixes ### Fixes * Backport scoped `allowed-tools` grant handling from #768: recognize declarations such as `Bash(git:*)`, preserve spaces and commas inside scopes, and keep malformed grants or embedded tool names from declaring shell capability. Command restrictions remain intact in the manifest. +* Stop backtick words in Python docstrings from marking files partially inspected (`static_parse_limit`) after the 2.13.3 shell-ownership backport. A docstring or other bare string statement is discarded, so its own body bounds a command named in it, unless the module reads docstrings back through `__doc__` or `getdoc`. The string-value guard also treats non-executing `os` calls (such as `os.makedirs` and `os.kill`) and name attributes (such as `type(error).__name__`) as inert; shell sinks, reflection and object escapes keep the whole-source fallback. * Ask Claude models reached through OpenAI-compatible gateways for exactly one JSON object when the schema is bound as `json_schema`. Gateways can serve that format through a tool Claude may skip, and a prose answer is rejected as `llm_structured_response_invalid`. Validation is unchanged. * Report which check rejected a model answer. Ledger rows and logs for rejected structured responses carry a `diagnostic` built only from fixed codes (check, channel, completion label, JSON shape, schema paths and per-attempt checks), never model output. * Retain up to 64 OSV advisories per locked package (was 16), so a dependency such as `filelock` with 17 advisories no longer leaves `poetry.lock` partial with `output_limit`. Per-scan detail and result budgets are unchanged. diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index 09f9cef81..2807f2117 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -49,6 +49,7 @@ from skillspector.python_tokens import ( PythonLiteralSpans, python_browser_template_shell_text, + python_discarded_string_spans, python_string_value_bounds_allowed, ) from skillspector.python_tokens import python_literal_spans as _python_literal_spans @@ -3572,6 +3573,7 @@ def __init__( self._spans: PythonLiteralSpans | None = None self._spans_computed = False self._string_value_bounds_allowed: bool | None = None + self._discarded_strings: tuple[tuple[int, int], ...] | None = None self._constant_fields: tuple[tuple[int, int, str], ...] | None = None self._cached_word_start = -1 self._cached_word_end = -1 @@ -3620,6 +3622,20 @@ def is_string_delimiter(self, position: int) -> bool: start, end = _python_string_body(self._content, *owner) return not start <= position < end + def _discarded(self, owner: tuple[int, int]) -> bool: + """Return whether the string token *owner* is part of a discarded statement. + + Python evaluates and drops a docstring or other bare string statement, + so no later concatenation or shell call can extend its value, whatever + execution paths the rest of the module has. + """ + if self._discarded_strings is None: + self._discarded_strings = python_discarded_string_spans( + self._content, self._check_runtime + ) + index = bisect_right(self._discarded_strings, (owner[0], len(self._content))) - 1 + return index >= 0 and owner[1] <= self._discarded_strings[index][1] + def string_value(self, start: int, end: int) -> tuple[str, int, int] | None: """Return the proven string value that holds a shell word, and its span. @@ -3645,7 +3661,7 @@ def string_value(self, start: int, end: int) -> tuple[str, int, int] | None: ) and ( self._python_import_check is None or self._python_import_check(self._check_runtime) ) - if not self._string_value_bounds_allowed: + if not self._string_value_bounds_allowed and not self._discarded(owner): return None if self._constant_fields is None: self._constant_fields = python_constant_string_fields( diff --git a/src/skillspector/python_tokens.py b/src/skillspector/python_tokens.py index 3bc6d6eba..7d3957199 100644 --- a/src/skillspector/python_tokens.py +++ b/src/skillspector/python_tokens.py @@ -29,6 +29,8 @@ # The tokenizer and the compiler disagree about a carriage return that does # not start CRLF, so such source never acquires token ownership. _LONE_CARRIAGE_RETURN_RE = re.compile(r"\r(?!\n)") +# ``str.splitlines`` also ends lines at these characters; Python does not. +_NON_NEWLINE_LINE_BREAK_RE = re.compile("[\r\x0b\x0c\x1c\x1d\x1e\x85\u2028\u2029](? tuple[int, int]: return opener + width, end - width +def python_discarded_string_spans( + content: str, check_runtime: Callable[[], None] +) -> tuple[tuple[int, int], ...]: + """Return the source spans of string statements whose value Python discards. + + A docstring, or any other statement that is only a string literal, is + evaluated and dropped: no later concatenation, call or shell receives it. + Introspection can read a docstring back. The bare name ``__doc__`` reads + only the module docstring, which then proves nothing; an ``.__doc__`` + attribute, a ``"__doc__"`` string or a ``getdoc`` helper can reach any + docstring, so such a module, or one that does not parse, proves no span. + """ + # Line offsets below follow ``str.splitlines``; Python ends lines only at + # newlines, so other separators would shift every later span. + if len(content) > MAX_PYTHON_AST_SOURCE_CHARS or _NON_NEWLINE_LINE_BREAK_RE.search(content): + return () + check_runtime() + with _PYTHON_OWNERSHIP_PARSE_LOCK, warnings.catch_warnings(): + warnings.simplefilter("ignore") + parsed = parse_python_source(content, "") + tree = parsed.tree + if tree is None: + return () + check_runtime() + + def offset(line: int, byte_column: int) -> int | None: + column = parsed.character_column(line, byte_column) + return None if column is None else parsed.line_character_starts[line - 1] + column + + module_docstring = ( + tree.body[0] + if tree.body + and isinstance(tree.body[0], ast.Expr) + and isinstance(tree.body[0].value, ast.Constant) + and isinstance(tree.body[0].value.value, str) + else None + ) + module_docstring_read = False + statements: list[ast.Expr] = [] + for node in ast.walk(tree): + check_runtime() + if ( + isinstance(node, ast.Attribute) + and node.attr in {"__doc__", "getdoc"} + or isinstance(node, ast.Name) + and node.id == "getdoc" + or isinstance(node, ast.Constant) + and node.value == "__doc__" + ): + return () + if isinstance(node, ast.Name) and node.id == "__doc__": + module_docstring_read = True + elif ( + isinstance(node, ast.Expr) + and isinstance(node.value, ast.Constant) + and isinstance(node.value.value, str) + ): + statements.append(node) + spans: list[tuple[int, int]] = [] + for node in statements: + if ( + module_docstring_read + and node is module_docstring + or node.end_lineno is None + or node.end_col_offset is None + ): + continue + start = offset(node.lineno, node.col_offset) + end = offset(node.end_lineno, node.end_col_offset) + if start is not None and end is not None: + spans.append((start, end)) + return tuple(sorted(spans)) + + def python_string_value_bounds_allowed( content: str, check_runtime: Callable[[], None], @@ -1003,7 +1079,37 @@ def python_string_value_bounds_allowed( "R_OK", "W_OK", "X_OK", + # File, directory and signal operations run no program and take no command text. + "chdir", + "chmod", + "get_terminal_size", + "getegid", + "geteuid", + "getgid", + "getlogin", + "getpid", + "getppid", + "getuid", + "isatty", + "kill", + "link", + "makedirs", + "mkdir", + "readlink", + "remove", + "removedirs", + "rename", + "renames", + "replace", + "rmdir", + "symlink", + "umask", + "unlink", + "urandom", + "utime", } + # Name and path strings cannot reach another object or module. + name_dunders = {"__name__", "__qualname__", "__module__", "__file__"} shells = { "sh", "bash", @@ -1167,7 +1273,11 @@ def safe_attribute(module: str, attribute: str, node: ast.AST) -> bool: # A safe first attribute does not own the rest of a chain, e.g. # os.path.os.system or an event loop's subprocess_shell method. if ( - (node.attr.startswith("__") and node.attr.endswith("__")) + ( + node.attr.startswith("__") + and node.attr.endswith("__") + and node.attr not in name_dunders + ) or node.attr in reflection or node.attr in { diff --git a/tests/nodes/analyzers/test_python_discarded_string_ownership.py b/tests/nodes/analyzers/test_python_discarded_string_ownership.py new file mode 100644 index 000000000..7a2f70075 --- /dev/null +++ b/tests/nodes/analyzers/test_python_discarded_string_ownership.py @@ -0,0 +1,156 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""A discarded string statement cannot carry shell operands to later execution. + +The 2.13 shell-ownership backport checks backtick words inside Python strings +as their own candidates. In a module with any recognized shell path, string +values keep the whole-source fallback, so a docstring such as +``Use `tool` on PATH`` made ordinary helpers partially inspected. A docstring +is evaluated and dropped, so its own body bounds those operands unless the +module reads docstrings back. +""" + +from __future__ import annotations + +import pytest + +from skillspector.inspection_ledger import LedgerOutcome +from skillspector.nodes.analyzers import static_patterns_tool_misuse as tm +from skillspector.nodes.analyzers import static_runner +from skillspector.python_tokens import ( + python_discarded_string_spans, + python_string_value_bounds_allowed, +) + +_SHELL_SINK = ( + "import subprocess\n\n\ndef run(cmd):\n return subprocess.run(cmd, shell=True)\n\n\n" +) +_PADDING = "".join(f"value_{index} = {index}\n" for index in range(300)) +_DOCUMENTED = ( + 'def find():\n """Use TOOL_BIN when set, else `tool` on PATH, or None."""\n return None\n' +) + + +def _exhausted(source: str) -> bool: + return tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="python") + + +@pytest.mark.parametrize( + "docstring", + [ + pytest.param('"""Use TOOL_BIN when set, else `tool` on PATH, or None."""', id="function"), + pytest.param( + '"""Append `data` to `path`, up to `limit` bytes.\n\n Whole lines are kept."""', + id="multiline", + ), + pytest.param('"Register the `sync` subcommand."', id="single-quoted"), + ], +) +def test_docstring_command_words_complete_beside_a_shell_path(docstring: str) -> None: + source = ( + _SHELL_SINK + + f"def helper():\n {docstring}\n return None\n" + + 'options = ["--recursive", "--force", "/tmp/build"]\n' + + _PADDING + ) + assert not python_string_value_bounds_allowed(source, lambda: None) + assert not _exhausted(source) + + +def test_documented_helper_reaches_completed_ledger_outcome() -> None: + path = "scripts/helper.py" + source = _SHELL_SINK + _DOCUMENTED + 'options = ["-rf", "/tmp/x"]\n' + _PADDING + result = static_runner.run_static_patterns_with_ledger( + {"components": [path], "file_cache": {path: source}}, [tm] + ) + row = result["inspection_ledger"][0] + assert row["outcome"] is LedgerOutcome.COMPLETED + assert "reason_code" not in row + + +@pytest.mark.parametrize( + "tail", + [ + pytest.param("run(find.__doc__ + ' -rf /')\n", id="doc-attribute"), + pytest.param("run(getattr(find, '__doc__') + ' -rf /')\n", id="doc-getattr"), + pytest.param("import inspect\nrun(inspect.getdoc(find) + ' -rf /')\n", id="inspect-getdoc"), + ], +) +def test_docstrings_read_back_keep_the_whole_source_fallback(tail: str) -> None: + assert _exhausted(_SHELL_SINK + _DOCUMENTED + tail + _PADDING) + + +def test_module_docstring_read_through_doc_name_keeps_the_fallback() -> None: + source = '"""Use `tool` on PATH."""\n' + _SHELL_SINK + "run(__doc__ + ' -rf /')\n" + _PADDING + assert _exhausted(source) + + +def test_destructive_operands_inside_a_docstring_remain_partial() -> None: + source = _SHELL_SINK + 'def helper():\n """Then `pick` -rf /"""\n' + _PADDING + assert _exhausted(source) + + +def test_assigned_string_flowing_to_a_shell_keeps_the_fallback() -> None: + source = _SHELL_SINK + 'command = f"Report `{tool}`"\ncommand += " -rf /"\nrun(command)\n' + assert _exhausted(source + _PADDING) + + +def test_discarded_string_spans_cover_only_string_statements() -> None: + source = ( + '"""Module `x`."""\n' + "import argparse\n" + "def f():\n" + ' """Run `tool` on PATH."""\n' + ' value = "kept `y`"\n' + ' "stray `z`"\n' + " é = 1\n" + ' "after `é`"\n' + "argparse.ArgumentParser(description=__doc__)\n" + ) + spans = python_discarded_string_spans(source, lambda: None) + assert [source[start:end] for start, end in spans] == [ + '"""Run `tool` on PATH."""', + '"stray `z`"', + '"after `é`"', + ] + + +@pytest.mark.parametrize( + "source", + [ + pytest.param('def f(:\n """`x`"""\n', id="syntax-error"), + pytest.param('def f():\n """`x`"""\n\x0c\nprint(1)\n', id="form-feed"), + pytest.param('def f():\r """`x`"""\r', id="lone-carriage-return"), + ], +) +def test_unprovable_sources_have_no_discarded_spans(source: str) -> None: + assert python_discarded_string_spans(source, lambda: None) == () + + +@pytest.mark.parametrize( + "use", + [ + "import os\nos.makedirs('out', exist_ok=True)\n", + "import os\nos.kill(1, 0)\n", + "import os\nos.readlink('current')\n", + "try:\n pass\nexcept Exception as error:\n print(type(error).__name__)\n", + "import module\nprint(module.__file__)\n", + ], +) +def test_non_executing_calls_and_name_attributes_keep_string_bounds(use: str) -> None: + assert python_string_value_bounds_allowed(use, lambda: None) + + +@pytest.mark.parametrize( + "use", + [ + "import os\nos.system(command)\n", + "import os\nos.putenv('PATH', value)\n", + "value = object()\nprint(value.__class__)\n", + "value = object()\nprint(value.__dict__)\n", + "value = object()\nprint(value.__globals__)\n", + ], +) +def test_shell_paths_and_object_escapes_still_disable_string_bounds(use: str) -> None: + assert not python_string_value_bounds_allowed(use, lambda: None)