ADR-018: No third-party text in the error envelope#

Status#

Accepted

Context#

Third-party exception strings were reaching both the HTTP response body and the logs from three sites: litellm provider exceptions (LiteLLMClient._complete_once), pydantic ValidationError on a malformed LLM response, and SQLAlchemy connectivity errors in SqlAlchemyUsageRepository. All three call str(exc) and either return it in a response message or log it verbatim.

That text is provider-controlled and unbounded. litellm embeds the api_base/deployment URL, model name and Azure content-filter verdicts; pydantic v2 ValidationError embeds input_value — the model’s raw response text, which docs/operations/observability.md lists as a hard prohibition; SQLAlchemy connectivity errors routinely embed the DSN, password included when it is in the URL.

Two complications rule out a narrow fix:

  • Sanitizing the message is not sufficient. Every translation site raises ... from exc, so the third-party exception stays attached as __cause__. Any handler that logs with exc_info=True or logger.exception emits the chained traceback, which carries the original text regardless of what the message says.

  • DEBUG-gating is not protective. LOG_LOGLEVEL defaults to DEBUG for application packages, so “log the detail at DEBUG” ships on by default.

Removing the free text also removes real signal: no handler was registered for LLMRateLimitError, LLMBadRequestError or LLMContentPolicyViolationError — all three collapsed into a generic 502, so the provider string was the only way a caller could distinguish “the content filter rejected this” from “the provider is down.”

Decision#

Error signal derives from the exception type, never from third-party text. A third-party exception string may be read exactly once, at the adapter boundary, to choose a domain error class. It is never stored on the error, returned to a caller, or logged verbatim.

  1. Response bodies and logs get asymmetric treatment. No provider text crosses the trust boundary into a response, ever — response messages for provider-derived errors are fixed, hand-written constants. Logs keep diagnostic detail (exception type, provider status code, model), because ops needs it to triage a 502.

  2. The log sink inherits the corpus’s data classification. raise ... from exc is kept for debuggability, which means a traceback emitted via exc_info=True / logger.exception may still contain provider-controlled text through __cause__. That is accepted deliberately: log output is in scope for data classification and must not be exported to third-party log analytics without review.

  3. A handler may echo str(exc) only when the message was authored in this repo. AnalysisError / AnalysisTimeoutError messages are all literal constants (or a literal plus a formatted float) raised from qfa.services, so _handle_analysis_error echoing str(exc) stays safe. Provider-derived classes (LLMError and subclasses, PromptInjectionDetectedError, UsageRepositoryUnavailableError) never get this treatment — even their in-repo domain-error message is not echoed by the corresponding handler, so a diagnostic string kept for the logs (e.g. the injection pattern name) can’t leak into a response by a future edit.

  4. Classified scalars replace free text where callers need signal. LLMError carries provider_status: int | None; LLMRateLimitError additionally carries retry_after: int | None, read only from the provider’s Retry-After response header — never from exception text. PromptInjectionDetectedError(AnalysisError) gives prompt-injection detection its own type instead of a substring match on the message.

  5. The Azure content-filter sniff is the one sanctioned read of a provider string. "filtered" in msg and "content management policy" in msg inspects the litellm BadRequestError message to choose between LLMContentPolicyViolationError and LLMBadRequestError — but the string itself is never propagated into either error.

Rejected options#

  • Replacing the Azure content-filter sniff with structural detection (litellm classification or innererror.code) — couples the adapter to provider internals for a bigger test surface, for a problem the string sniff already solves; out of scope for this change.

  • Denylist/regex scrubbing of provider strings, in either sink — this is pattern-filtering of untrusted, unbounded, provider-controlled text and fails open on anything unanticipated.

  • A “verbose errors” opt-in setting that echoes provider detail behind a flag — a switch that can be, and eventually will be, left on in production.

  • Severing the from exc chain — the debuggability loss is not worth the containment, and it breaks the exception-chaining idiom used elsewhere in the codebase.

Consequences#

  • New HTTP mapping: LLMContentPolicyViolationError → 422 content_policy_violation; LLMRateLimitError → 429 llm_rate_limited (with Retry-After); LLMTimeoutError → 504 llm_timeout; LLMBadRequestError and LLMError → 502 llm_error; PromptInjectionDetectedError → 422 prompt_injection_detected. See docs/architecture/04-crosscutting.md for the authoritative table.

  • _handle_analysis_error’s "injection" in str(exc).lower() routing is deleted — it was also a latent bug, since rewording the injection message would have silently turned a 422 into a 502.

  • No new setting or environment variable: RATE_LIMIT_RETRY_AFTER_FALLBACK_SECONDS is a module constant in qfa.api.app, not a field on qfa.settings.

  • No response body field other than error.message changes shape, and no success-path response changes at all.

When to revisit#

  • If a fourth translation site starts propagating third-party text, apply the same pattern: fixed message, classified scalars, from exc kept.

  • If the Azure content-filter signal needs to become more precise than a substring match, that is the trigger to revisit the “rejected: structural detection” option above — not to loosen the propagation rule.

Participants#

Marius