#34940 widened the mask-in-place safety guard so a Responses-API
`instructions` field (and a combined messages+input body) skips the
PII masking branch. With `on_flagged: "monitor"` that fell straight
through to "allow", so PII that used to be masked now reaches the
model unredacted.
Monitor means "don't block", not "don't redact". Recover the one shape
whose payload is still fully writable: mask it and write the redacted
instructions back into `data["instructions"]` directly, since
apply_redacted_messages_back has no path for that field and would
otherwise fold the instructions text into `data["input"]`.
The combined messages+input and multimodal shapes stay unmasked - both
are unsafe to write back, not merely unwritable - and now log an error
naming the reason instead of passing silently.
No block/allow decision changes: block and inject_system_message keep
the exact outcomes #34940 shipped.
A guardrail block or failed scan that fires after SSE chunks have been
flushed can no longer set an HTTP status, so raising HTTPException there
silently truncated the stream. _emit_streaming_http_error now routes
post-flush failures through the endpoint translation's
build_stream_error_items, emitting the surface-correct error frame on
chat completions (data: {error}), /v1/messages (event: error), and
/v1/responses (ErrorEvent with the next sequence number). Pre-flush
blocks still raise with a real HTTP status.
Successful flags-on scans also logged metadata.guardrail_information as
null: the chat handler planted litellm_metadata on a route whose bucket
is metadata, flipping the bucket for every later write, and responses
streams fired their spend log before the eos scan ran. The chat handler
now merges user_api_key metadata through get_or_create_metadata_bucket,
and deferred stream-complete logging is armed for aresponses like it
already was for anthropic_messages.
* feat(guardrails): honor Lakera v2 skip-message flags and add advisory (inject_system_message) mode
Squashed rebase of bugfix/lakera-v2-skip-system-tool-messages onto latest
litellm_internal_staging (900+ commits ahead; a commit-by-commit rebase hit
repeated conflicts against the same files across earlier review-round
commits, so the branch's cumulative diff was reapplied in one pass instead).
Adds skip_system_message_in_guardrail/skip_tool_message_in_guardrail support
to Lakera v2, a third on_flagged: "inject_system_message" advisory mode, and
the associated masking-safety-guard hardening (multimodal content, non-
maskable message fields, combined messages+input, and structured Responses-
API input in advisory delivery) found across this PR's review rounds.
* fix(guardrails): don't let one invalid guardrail config crash proxy boot
init_guardrails_v2 had no try/except around initialize_guardrail, so a
guardrail whose litellm_params fail validation at construction time (for
example Lakera's on_flagged=inject_system_message combined with
mode=during_call, or a malformed advisory_system_message template) raised
uncaught and crashed the entire proxy at startup, taking down every other,
correctly-configured guardrail in the list. Catch ValueError/TypeError per
guardrail, log a warning, and skip it, matching the same pattern already
used for the DB-driven guardrail-creation path in guardrail_endpoints.py.
* fix(guardrails): preserve message fields and mask PII before advising in Lakera v2
Mask-in-place degraded to a hard block for any message carrying a field
beyond role/content (tool_call_id, tool_calls, name, cache_control), for a
message excluded by skip_system_message_in_guardrail/skip_tool_message_in_guardrail,
or for a message with no inspectable text, since it rewrote data["messages"]
wholesale from a synthetic role/content-only list built for the Lakera API
call. That made masking effectively unusable for any real tool-calling
conversation and made the skip flags flip every PII-only violation to a hard
block instead of masking just the in-scope text.
Replace the wholesale rewrite with a scope-index merge, reusing the same
merge_guardrailed_scoped_messages helper the OpenAI/Anthropic guardrail
translation handlers already use for this: patch content in place on a copy
of each original message actually sent to Lakera, and leave every
skipped/no-text/out-of-scope message untouched at its original position.
This also fixes on_flagged="inject_system_message" (advisory mode) shipping
raw unmasked PII to the model: a PII-only violation is now masked the same
way regardless of on_flagged, and the advisory note is reserved for flags
masking can't resolve on its own.
Addresses maintainer-reported regressions on BerriAI/litellm#34940.
* fix(guardrails): satisfy new lint gates for the masking/advisory fix
Parameterize the write-back helper's dict param and suppress the two new
lint rules that landed on the base while this branch was in flight: TQ008
(patching an internal collaborator) for two pre-existing tests unrelated to
this change, and LIT001 for a param that genuinely needs to mutate the
caller's request dict in place.
* fix(guardrails): normalize role casing in Lakera v2 masking scope, log skipped guardrails louder
Greptile finding: the masking scope helper compared roles case-sensitively
while filter_messages_by_skip_flags (used to build what's actually sent to
Lakera) normalizes casing, so an uppercase-cased "System"/"TOOL" role
survived the scope filter but was excluded from the inspected list. The
resulting length mismatch raised inside the strict positional zip, turning
a maskable PII-only violation into an unhandled request failure. Lowercase
the role comparison to match.
Also, per veria-ai's finding that a skipped invalid guardrail now fails
open: log it at error level with an explicit note that the proxy is
starting without that guardrail, so it's not mistaken for routine info.
* fix(guardrails): mask maskable PII in mixed violations before advising in Lakera v2
on_flagged="inject_system_message" only masked when a violation was
PII-only; a mixed violation (PII plus a non-PII flag like prompt injection)
fell straight through to the advisory branch with the raw PII still in
place, in both async_pre_call_hook and async_moderation_hook. Mask whatever
Lakera returned location data for before appending or logging the advisory,
so a mixed violation never ships raw PII just because something else was
also flagged.
Also degrade to blocking, same as block mode already does, when nothing
can be safely masked at all (multimodal content, or messages combined with
a Responses API input field) instead of showing an advisory note next to
raw, unredacted content.
Widened call_v2_guard/_mask_pii_in_messages/the write-back helper's message
parameters from list to Sequence to match what's actually passed through
from _filter_skipped_messages, instead of duplicating list(...) casts at
every call site.
* fix(guardrails): don't hard-block advisory mode for non-PII flags on non-maskable input
Bugbot finding: gating the entire inject_system_message branch on
is_multimodal_input hard-blocked every flagged request on Responses
instructions, combined messages+input, or multimodal content, including
a prompt-injection-only violation with no PII at all. Masking safety only
matters when there's actual PII to mask; a violation with no PII needs no
masking, so the advisory should still be delivered normally.
Only degrade to blocking when the breakdown actually contains a PII
detection and masking isn't safely possible. Otherwise, mask whatever's
maskable (if any) and deliver the advisory as before.
* fix(guardrails): require payload and breakdown for Lakera v2 advisory mode
Advisory mode's mixed-violation masking safety net can only redact
detected PII when Lakera's response carries both the breakdown (to
detect a PII hit at all) and payload (the location data to mask by).
payload=False or breakdown=False alongside on_flagged='inject_system_message'
silently forwarded raw PII next to the advisory note. Reject that
combination at construction and hot-reload time instead.
* fix(guardrails): skip_system_message_in_guardrail must not force-block Lakera masking
_has_responses_instructions treated any non-empty data["instructions"]
as unsafe to mask regardless of skip_system_message_in_guardrail, even
though that flag excludes the instructions-derived synthetic system
message from what Lakera ever inspects. PII detected purely in the
maskable non-system content was force-blocked instead of masked.
Also fixes pre-existing LIT010 (missing Final) violations in
_has_responses_instructions, _breakdown_has_pii_violation, and
async_post_call_success_hook that the rebase's lowered budget ceiling
now flags.
* chore: retrigger CI (GitHub Actions runner-acquisition failure on prior push)
* fix(guardrails): address maintainer review findings on Lakera v2 advisory mode
- Gate advisory_system_message template validation on on_flagged=
'inject_system_message', since block/monitor mode never reads it.
- Allow on_flagged='inject_system_message' with mode='during_call' at
construction/hot-reload instead of rejecting it; async_moderation_hook
already degrades gracefully (masks if possible, else logs a warning).
- reinitialize_guardrail now restores the previous live instance when the
new config fails to initialize, instead of leaving the guardrail deleted
entirely with nothing enforcing it.
- PATCH /guardrails/{id} rolls back the DB write and returns 422 when the
in-memory sync rejects the new config, instead of persisting a config
that never actually took effect and returning 200.
- Qualifire now rejects on_flagged values it doesn't implement (only
Lakera should accept 'inject_system_message'; LitellmParams flattens
the field across every guardrail config mixin).
* fix(tests): satisfy lint gates and update collateral test for advisory-mode fixes
- Add match= to a too-broad pytest.raises(ValueError), and suppress the
new TQ008 mocker.patch findings (same pattern already used by sibling
scenarios in this test).
- test_init_guardrails_v2_skips_invalid_guardrail_instead_of_crashing_boot
used mode='during_call' + on_flagged='inject_system_message' as its
invalid-config example; that combination is now accepted, so swap in
the payload/breakdown-missing case and add a test confirming during_call
advisory mode constructs successfully.
* docs(CLAUDE.md): auto-capture review learnings without being asked
This session found three real bugs a human maintainer caught after eight
rounds of bot review and live-proxy verification all missed them. Add a
standing instruction to write learnings.md entries the moment a root
cause is understood, in both the repo-wide file and any relevant skill's
own file, instead of relying on being asked.
* feat(guardrails): add scan_raw_request flag so YAML order can't change enforcement
Maintainer finding on BerriAI/litellm#34940: guardrails for the same hook
run sequentially over one shared, progressively-mutated request dict, so
declaring a masking guardrail before a blocking one hides the violation
from it (200 vs 400 depending purely on YAML order).
scan_raw_request opts a guardrail into always evaluating a snapshot taken
before any guardrail in the hook ran, regardless of its declared position.
Same contract as run_in_parallel: block-only, its own mutations discarded.
Verified live: real proxy, real Gemini call, two custom guardrails (a
redactor then a blocker). Same request, same declared order -- without the
flag the blocker never sees the raw secret (200); with it, the blocker
correctly rejects before any provider call (400).
* fix(guardrails): harden scan_raw_request against review findings
- Use safe_deep_copy instead of a bare deepcopy for the raw-request
snapshot; request payloads commonly carry unpicklable objects (e.g. an
otel span in metadata), which previously raised on every guarded
request when tracing was enabled (Bugbot, High).
- Only compute the snapshot when a guardrail actually opted in, and take
it before _maybe_execute_pipelines runs, so a pipeline-mutated payload
can't hide a violation from a scan_raw_request guardrail outside the
pipeline (veria-ai).
- Log a warning when a scan_raw_request guardrail returns a modified
payload, since that mutation is discarded and the combination is
otherwise silently exploitable for a masking-capable integration
misconfigured this way (veria-ai).
* chore(openapi): regenerate lazy snapshot and dashboard schema types
The lazy OpenAPI snapshot (litellm/proxy/_lazy_openapi_snapshot.json) and
the derived dashboard schema.d.ts had drifted stale relative to the
guardrail config model changes across this PR's rounds (advisory mode,
scan_raw_request, and upstream additions picked up by rebasing).
Regenerated via the CI's own documented fix:
uv run python -m litellm.proxy._lazy_openapi_snapshot
npm run gen:api (via make check)
* chore(openapi): pick up cache_hit_filter field after rebase
* fix(guardrails): stop scan_raw_request warning from firing on every call
_process_guardrail_callback always returns a dict once a guardrail runs
(mark_pre_call_hook_ran unconditionally stamps bookkeeping metadata), so
comparing the result to non-None warned on every request even when the
guardrail never touched the payload. Compare against a bookkeeping-only
baseline instead, so only an actual content mutation triggers the warning.
* fix(guardrails): make scan_raw_request snapshots independent of safe_memory_mode
safe_deep_copy can return the original object under litellm.safe_memory_mode,
or alias a per-key reference on copy failure. Under that mode, the
scan_raw_request comparison baseline aliased raw_request_snapshot (and
therefore the live request), letting mark_pre_call_hook_ran write a
premature execution marker that a deployment-level guardrail sharing the
same name would read as "already ran" and skip. Also affected the feature's
core isolation guarantee: input_data itself could alias the live request
under the same mode. Replace every scan_raw_request snapshot with
_independent_snapshot, which never returns an alias, only a genuine copy
or None.
* fix(guardrails): gate during_call mixed-violation masking behind an actual PII check
The during_call branch for a mixed violation under on_flagged=inject_system_message
unconditionally masked and reassigned data["messages"], even for a pure
prompt-injection violation with zero PII, unlike async_pre_call_hook which
already gates the same call behind _breakdown_has_pii_violation. The
unconditional reassignment touched shared request state during a hook
documented as racing with the concurrent LLM dispatch, for no reason when
there was nothing to mask.
* fix(guardrails): stop scan_raw_request from silently no-op'ing on real requests
_independent_snapshot did one whole-dict copy.deepcopy and returned None on
any failure. Every real proxy request carries data["litellm_logging_obj"]
(a Logging instance nesting a live OTel span with a real lock) by the time
pre_call_hook runs, which can never be deep-copied, so the snapshot failed
on every real request and silently fell back to the live, unisolated data
with no warning -- defeating the entire feature in production while every
existing test (none of which set litellm_logging_obj) kept passing.
Rework the helper to deep-copy each top-level key independently, falling
back to the original reference only for the specific key that fails, same
crash tolerance as safe_deep_copy's own per-key fallback. It never returns
None now; only the keys scan_raw_request actually depends on (messages/
input, metadata/litellm_metadata) need to be genuinely independent.
* fix(guardrails): block during_call when PII can't be safely masked
Greptile finding (P1, security): async_moderation_hook's inject_system_message
branch had no equivalent to async_pre_call_hook's degrade-to-blocking case for
a PII violation on input that can't be safely masked (e.g. combined
messages+input). It fell through to the advisory no-op branch and let raw,
unredacted PII reach the model with no protection at all. Raising still
blocks the response from reaching the caller even though during_call races
with the LLM dispatch, the same mechanism on_flagged="block" already relies
on for this hook, so add the same block-instead-of-advisory branch pre_call
already has.
* chore(lint): fix LIT002 ceiling after rebase merge conflict resolution
* fix(lint): suppress genuine LIT002 hits instead of padding the ceiling
My earlier rebase conflict resolution for type-discipline-budget.json's
LIT002 limit was too low, then overcorrected by padding it well above the
actual measured count. Root-caused instead: _independent_snapshot and the
PATCH-endpoint rollback path legitimately construct plain, mutable
request-payload/config dicts (matching this file's existing precedent for
the same shape), so suppress those four sites with `# mutable-ok:` rather
than reshaping code that must stay a plain dict by contract. Set the limit
to the exact current measured total; the small remaining gap vs upstream's
own committed ceiling is pre-existing drift in litellm_internal_staging
itself (its own tree already measures over its committed limit), not
attributable to this PR.
* fix(guardrails): stamp live request when a scan_raw_request guardrail runs
_run_sequential_guardrail_callback and _run_parallel_pre_call_guardrails only
called mark_pre_call_hook_ran on throwaway snapshot copies for a
scan_raw_request guardrail, never on the live request returned to the
caller. A later async_pre_call_deployment_hook (router-level guardrail
re-check) reads that marker on live kwargs to decide whether to skip
re-running the same guardrail; since it was never stamped there, the
guardrail ran a second time on live data, doubling the external call and
re-applying whatever scan_raw_request's contract says should be discarded.
* fix(guardrails): revalidate Qualifire's on_flagged on live config reload
on_flagged was validated only in __init__. The base
CustomGuardrail.update_in_memory_litellm_params is a generic setattr loop
with no revalidation, so a live config update (PUT /guardrails/{id}, no
restart) could setattr on_flagged="inject_system_message" onto a running
instance, bypassing the constructor's rejection -- silently blocking every
flagged request under an "advisory" label. Mirrors LakeraAIGuardrail's own
update_in_memory_litellm_params override added earlier in this PR.
* fix(guardrails): honor scan_raw_request for pipeline-managed guardrails
A scan_raw_request=True guardrail that is itself a pipeline step never saw
raw_request_snapshot: PipelineExecutor.execute_steps had no way to receive
it, and pipeline-managed guardrails are fully excluded from the normal
sequential/parallel loops that implement the flag. Such a guardrail silently
evaluated whatever an earlier pass_data step in the same pipeline had
already rewritten, defeating the flag for pipeline-managed guardrails.
Moves the snapshot helper (renamed independent_snapshot) from proxy/utils.py
to litellm_core_utils/core_helpers.py so pipeline_executor.py can use the
same independent-copy logic without a circular import, threads
raw_request_snapshot through _maybe_execute_pipelines and
PipelineExecutor.execute_steps/_run_step, and discards a scan_raw_request
step's returned data the same way the sequential/parallel loops already do.
* chore(openapi): pick up upstream drift after rebase onto litellm_internal_staging
* fix(guardrails): stop attempting PII masking during during_call in Lakera v2
Greptile finding (P1, security): during_call runs concurrently with the LLM
dispatch. In the common path, the provider call already binds its messages
kwarg before this guardrail's coroutine gets a chance to run, let alone
before its own network round trip to Lakera completes -- masking here can
never reliably reach the outgoing request, and _apply_redacted_messages_back_
preserving_fields reassigns to a new list object rather than mutating in
place, so even winning the race wouldn't help. This affected both the
PII-only and mixed-violation masking branches, all added in this same PR.
Remove masking from async_moderation_hook entirely and let PII violations
fall through to the normal on_flagged branching: block under "block" or
"inject_system_message" (extending the existing multimodal-only block to
cover every PII case, since masking is proven non-functional regardless of
input shape), log-and-allow under "monitor" -- consistent with how every
other violation type in this hook is already handled.
---------
Co-authored-by: Deepanshu <deepanshu.lulla@alpha-sense.com>
* fix(guardrails): add fail-open mode to CrowdStrike AIDR guardrail
Add a fail_on_error param (default True, preserving existing behaviour) to
the CrowdStrike AIDR guardrail, mirroring model_armor and generic_guardrail_api.
When fail_on_error=False the guard fails open only on server errors (5xx) and
connectivity failures, so the request proceeds unmodified. Caller-controlled
4xx responses and result.blocked policy blocks always fail closed. The
applied-guardrails header is recorded even on the fail-open path.
* fix(guardrails): fail open AIDR 4xx
* refactor(guardrails): isolate AIDR fail-open
* style(guardrails): format AIDR fail-open
* ci: satisfy unit workflow timeout invariant
* refactor(guardrails): accept AIDR mappings
* test(guardrails): inject AIDR HTTP client
* fix(guardrails): harden AIDR fail-open against delivered verdicts and record fail-open status
Reads the blocked verdict from the raw body before guard_output validation so schema drift or a changed verdict type cannot fail open past a delivered block. A transformed response that cannot be parsed fails closed so delivered redactions are never dropped. Fail-open runs record guardrail_status guardrail_failed_to_respond with timings instead of success. Restores the fail-open behavior tests dropped mid-PR and reverts the payload Mapping widening
* test(guardrails): cover fail_on_error wiring and fail-closed default for CrowdStrike AIDR
* chore(guardrails): annotate the transformed-drift detail payload for the LIT002 budget
---------
Co-authored-by: abrekhov <abrekhov@users.noreply.github.com>
Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
* fix(presidio): chunk oversized text before /analyze so large content blocks do not fail
The Presidio PII guardrail sent each content block to the analyzer as a
single /analyze call with no size check. Analyzer deployments commonly cap
the request body (the reporting deployment rejects bodies over 1,000,000
bytes with HTTP 413), so large blocks failed closed, and analyzer latency
grew linearly with payload size.
analyze_text now splits texts larger than presidio_analyze_chunk_size_bytes
(default 500,000 UTF-8 bytes, configurable per guardrail) into overlapping
chunks, analyzes them concurrently, remaps each detection's start/end onto
the original text, and deduplicates detections from the overlap regions.
Anonymization, blocked-entity checks, score filtering, numbered-token
unmasking, telemetry, and the dashboard entity positions all consume the
remapped global offsets unchanged.
Resolves LIT-4785
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(presidio): review-round hardening for chunked analyze
- measure the chunk budget on the JSON-serialized text (non-ASCII escapes
expand beyond raw UTF-8, so a raw-byte budget could still exceed the
analyzer body limit)
- share the chunk fan-out semaphore per event loop and instance instead of
per call, so many oversized blocks cannot multiply concurrent analyzer
calls
- apply configured score thresholds and deny list per chunk BEFORE overlap
resolution, so a below-threshold span cannot displace a detection the
thresholds keep
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
`requests` has no default timeout, so a host that accepts the connection and
never answers blocks the calling thread forever.
The one on the request path is the HiddenLayer guardrail's `_get_jwt`. It runs
synchronously inside `_call_hiddenlayer` whenever the hour-long JWT expires and
the API answers 401, so a stalled auth host parked the worker's whole event
loop, not just the guarded request. The other eight are the teams and users CLI
clients, which pin the operator's terminal instead.
`TeamsManagementClient` and `UsersManagementClient` now take the same
`timeout: int = 30` their `HTTPClient` sibling already had, and `Client` threads
its own timeout down to teams. `_poll_for_ready_data` already passed a timeout
through a TypedDict that ruff could not see into; passing the argument directly
retires both the TypedDict and the suppression it would have needed.
Graduate S113 into ruff.toml so the next `requests` call without a timeout fails
the lint step.
Every repository handed its `.table` back untyped, so a dozen modules had
each grown a private `_PrismaTableActions` Protocol to paper over it. They
had drifted: some declared `update` as returning the row, others the row or
None, and none agreed on whether `find_many` was covariant
Replace all of them with a single `TableActions[RowT_co]` in
`litellm/repositories/prisma_protocols.py`, keyed to the prisma row each
repository is bound to. Query inputs stay `Mapping[str, object]` so callers
keep passing plain dicts, and `find_many` returns `Sequence` so the row type
stays covariant
Typing the nullable returns honestly surfaced paths that were already
crashing. A team admin could never edit or delete a memory entry owned by
their team: the write-auth check fed a raw prisma row to a helper that
expects the domain model, so `members_with_roles` arrived as plain dicts and
the request died as a 500 instead of applying the edit. Non-admin members hit
the same 500 in place of the 403 they were owed, so refusal and breakage were
indistinguishable. `/v2/model/info?user_models_only=true` dereferenced a
missing user row rather than returning the 400 the route already had, three
team routes dereferenced a team deleted between the read and the write, and
the agent registry dereferenced a missing agent instead of naming it
basedpyright drops 2,132 errors, 1,454 of them reportAny and 73
reportExplicitAny. The dashboard's generated types pick up `string[]` where
they had `unknown[]` for a team's members, admins and models
The Presidio guardrail masks messages in place inside pre_call_hook, but three
paths independently persisted or emitted the raw pre-guardrail data: the
SpendLogs proxy_server_request body snapshot (taken before the hook runs),
a verbose_proxy_logger.debug dump of the raw request, and logging_only mode's
async_logging_hook, which never masked the model's response before it reached
external logging callbacks.
Resolves LIT-6015
* test: drop the cwd-relative sys.path.insert calls from the test suite
TQ003 stands at 1,077 across 1,058 files, and 1,015 of them are the same shape:
sys.path.insert(0, os.path.abspath("../..")) and its deeper siblings. The
argument resolves against the working directory rather than the file, so from
the repo root, where every job runs pytest, it inserts the directory two levels
above the checkout. It has never pointed at litellm. The package is installed
into the environment anyway, which is what actually makes the import work, and
what the rule's message has said all along.
Removing them leaves 1,634 imports of sys and os with no remaining reference,
and those go too, except where another test module imports the name back out of
the file. The rest of TQ003 is 62 call sites that resolve against __file__ or a
variable, which are a different question and are left alone.
Collection is identical either way: 45,871 tests and the same 51 pre-existing
collection errors before and after, and ruff reports no new undefined name.
* test: drop the duplicate imports the sys.path sweep exposed to F811
* test(pre-call-utils): restore the os import the new bedrock tests need
Twenty-three tests across eleven files opened with litellm.set_verbose = True
and never put it back, so the flag stayed on for everything that ran after them
in the same process. None of those files read the output it produces: no
caplog, no capsys, no assertion on a log line, so the flag was left over from
debugging. Deleting it beats restoring it, since restoring keeps the noise.
Ten of the eleven stop leaving the flag on. test_volcengine_embedding.py still
ends with it set, from something it exercises rather than from the test itself,
which is worth its own look.
Onyx, prompt security, hiddenlayer, repelloai and deepkeep all write straight to
os.environ and unset again at the bottom of each test. None of the five has a
try/finally, so the moment a test fails it returns to the runner with the keys
still set and whatever runs next in that worker inherits them.
Raising inside test_onyx_guard_with_custom_timeout_from_kwargs on the current
files leaves ONYX_API_BASE and ONYX_API_KEY behind; doing the same in
test_hiddenlayer_config_saas leaves HIDDENLAYER_API_BASE. Both come back clean
after this.
89 raw writes and the hand-rolled deletes become monkeypatch calls. The
class-level setup_method and teardown_method pair in the onyx file, sweeping the
same three keys twice, becomes one autouse fixture. The sys.path.insert lines
and their now-unused imports go too, and litellm.set_verbose = True, which only
turned global debug logging on for whatever ran next, is dropped rather than
restored.
test_onyx_guard_config and test_prompt_security_guard_config asserted nothing at
all, so they could only fail by raising. Each now pins what init_guardrails_v2
produces: exactly one guardrail of the right class on litellm.callbacks,
carrying the configured name, default_on and hook. The zero-assert tests in the
other three are left alone; those are a judgement about each guardrail rather
than a mechanical sweep.
tests/test_litellm/proxy/guardrails passes at 2873.
* test: use monkeypatch.setenv for env writes in tests/test_litellm
`os.environ["X"] = v` inside a test leaks the value into every test that runs
after it in the same worker, so ordering decides the result. 262 of those
writes across 40 files now go through pytest's `monkeypatch` fixture, which
restores the previous value at teardown.
The rewrite skips any test that a mock.patch-family decorator wraps, any test
with defaulted positional parameters, any test whose own name is called
directly elsewhere, and rebinds nothing inside nested defs, because in each of
those cases appending a fixture parameter changes what pytest or mock binds.
Ratchets the TQ004 ceiling from 768 to 506.
* fix(test): delete the key through monkeypatch instead of popping it first
Five tests popped a key straight out of `os.environ`, ran, then restored it with
`monkeypatch.setenv`. By the time monkeypatch saw the name it was already gone,
so it recorded "absent" as the value to go back to and deleted the key at
teardown. On a worker that inherited a real `RESEND_API_KEY`, `SENDGRID_API_KEY`,
`UI_PASSWORD`, `LITELLM_SALT_KEY` or `OPENAI_API_KEY`, every test after the first
one ran without it.
`monkeypatch.delenv(..., raising=False)` removes the key and restores whatever
was there, so the try/finally the manual restore needed goes with it.
* chore(test): leave the two cost-calc files to the PR that rewrites them fully
Both files are also in #37815, which converts the module-global writes as well
as the env writes and folds them into one fixture. Two PRs rewriting the same
lines differently is a conflict nobody benefits from resolving, so this one
drops back to staging on those two and keeps the other 39.
TQ004 clears 200 here instead of 275; the rest moves with #37815.
A name bound twice keeps only the second binding. In `tests/` that is nearly
always a repeated import, harmless but misleading, and the same rule is what
catches the cases that are not harmless: a local that shadows an import the
module still calls, and a second `def test_x` that quietly replaces the first.
311 of the 344 sites were repeated imports and came out with ruff's own fix.
The remaining 33 needed a decision. Four modules imported a name they never
used because a local definition below already shadowed it. Two comprehensions
bound `call` over `unittest.mock.call`, which those modules import and use.
One test rebound the two module handles its nested reload closure had captured.
One class attribute shadowed an unused `status` import.
The load-test fixtures move to a conftest, which is how pytest is meant to share
them, so the test module no longer imports three fixture names it never calls.
The nine `prisma_client` parameters keep a narrow `noqa`: pytest resolves that
fixture by name before the body runs, so the parameter never shadows anything.
`pytest.raises(Exception)` with no `match=` passes on any error that broad. A
TypeError from a refactor, a botched fixture, an import that moved: all of them
read as the rejection the test claims to police, so the test goes green for the
wrong reason and stays green after the behaviour it guards is gone.
PT011 closes that gap for the 317 sites B017 could not reach, because B017 only
fires on a single-statement body with no `as e` binding. Each pattern here is the
message the code actually raised, recorded by running the sites under a plugin
that logged the concrete type and text per call site, so the assertions describe
observed behaviour rather than a guess. Where a site raises more than one message
across its parametrize cases, the pattern is an alternation of what was seen;
where the exception carries an empty `str()` and puts the text on `.message`, the
site keeps a narrow `noqa` with the reason.
PT014 removes four parametrize cases that were listed twice. The duplicate re-runs
an assertion that already passed, and it usually marks a case someone meant to
vary and forgot to edit.
* test: enforce PT012 so a pytest.raises block cannot hide dead assertions
`with pytest.raises(...)` stops at the first statement that raises. Anything
sequenced after it inside the block never runs, so an assertion written there is
never checked and the test still reports green.
Two sites were doing exactly that, and both assertions turned out to be wrong
once they started running. tests/llm_translation/test_prompt_factory.py asserted
the bedrock rejection names "requires at least one non-system message", which
holds. tests/proxy_unit_tests/test_proxy_server.py asserted the prisma startup
failure mentions "httpx.ConnectError", which never appears: the failure is an
httpx.ConnectError whose message is "All connection attempts failed", so that
test now asserts the type. Its DATABASE_URL override moves to monkeypatch, since
the old restore sat below the assertion and leaked the invalid URL into every
later DB test the moment the assertion started being able to fail.
The remaining 72 sites are rewritten without changing what they exercise: setup
that cannot raise moves above the block, a nested `patch` moves outside it, and
bodies with real control flow (a stream drain, an if/else on sync_mode, a
retry loop) move into a local closure the block calls.
Fixing PT012 unmasked two B017s, since ruff only reports a blind
pytest.raises(Exception) once the block holds a single statement.
tests/proxy_unit_tests/test_auth_checks.py narrows to the ProxyException
can_key_call_model actually raises. tests/local_testing/test_completion_cost.py
was asserting vertex_ai/medlm-medium has no cost entry, which stopped being true
at some point; that dead first half is gone and the rest of the test, which
checks medlm pricing resolves above zero, now runs instead of being skipped.
* chore(ci): ratchet TQ004 to 768 after the prisma test moved to monkeypatch
* test(lint): ban blind pytest.raises(Exception) with ruff B017
A bare pytest.raises(Exception) accepts whatever the body throws. The TypeError
a refactor introduces satisfies it exactly as well as the rejection the test was
written for, so the crash reads as a pass and the test never goes red.
All 111 existing sites are narrowed here. A runtime probe recorded the concrete
exception each one actually catches, and each site now names that type. Where
the code under test genuinely raises a bare Exception, the site pins a stable
slice of the message with match= instead.
Two sites tell on themselves. The shared responses-API cancel test raises
"custom_llm_provider is required but passed as None" rather than talking to a
provider at all, because cancel_responses takes a provider, not a model. And
test_bedrock_guardrails_with_streaming was the only test in its file still
passing without AWS credentials, because the NoCredentialsError boto3 raised
long before the guardrail ran satisfied the blind raises.
* fix(test): widen the openai batch-dispatch assertion to OpenAIError
The narrowed NotFoundError only holds where OPENAI_API_KEY is set. Without one
the SDK raises OpenAIError while building the client, long before any 404, so CI
went red. OpenAIError covers both and still rejects a TypeError from a refactor.
`assert False` inside a `try:` raises AssertionError, which the `except
Exception` right below it catches, so several tests reported green no matter
what the code did. `pytest.fail` raises Failed, a BaseException, and escapes.
A bare `a == b` statement is evaluated and discarded. Nine of those sat in
tests, and one was comparing against a model name the router never produces.
Selects B011, B015, B018, PT015, PLR0133 and PLW0127 in ruff-tests.toml
alongside F821, with all 50 existing violations fixed, so no budget file or
ratchet is needed. CI already runs this config over tests/.
ruff.toml excludes tests/* from `ruff check`, so nothing has ever checked the
test tree for names that do not exist. That matters more in tests than in
product code: a NameError inside a test whose body is wrapped in
`except Exception: pass` is swallowed, and the test reports green forever.
Adds ruff-tests.toml selecting F821 alone, wired into the lint workflow and
`make lint-ruff`, and clears every existing violation:
- 4 tests interpolated an unbound `e` into a `pytest.fail` message reached only
on the failure path, so the NameError, not the assertion, is what ran.
test_llm_guard_error_raising is the worst: it passes today with content
safety disabled entirely. It now asserts the 400 and its detail body.
- 5 sites construct BaseExceptionGroup, a 3.11 builtin, in a tree that still
supports 3.10. Guarded behind the exceptiongroup backport that anyio already
pulls in below 3.11.
- 9 missing imports (json, openai, Any, Final, HTTPException), including one in
a helper that catches HTTPException by a name it never imported, so the
challenge path it exists to detect raises NameError instead.
- 5 annotations naming types imported inside the function body, hoisted to
module scope or TYPE_CHECKING.
- 2 blocks of dead code: everything after a pytest.fail in
test_claude_agent_sdk, and an unused helper in test_end_users calling a
function defined in a different module.
- 1 error-path f-string in the router-settings doc test that masked the real
FileNotFoundError behind a NameError.
Only F821 for now. Widening the select list means ratcheting thousands of
pre-existing findings, so rules go in one at a time with their violations
already fixed.
* feat(proxy): redact or drop individual batch records instead of rejecting the file
A single record tripping a guardrail rejected the whole upload, which is unusable for a file
holding thousands of rows. A record a guardrail rewrites is now submitted in its rewritten
form, a record it blocks is left out, and the create response reports every changed record by
both custom_id and line so a caller can reconcile against the file it sent. The same outcome
is written to the proxy log and to request metadata, so it is not visible only to the caller.
A rewritten record goes straight to a spool and only its offset is carried, so a masking
guardrail touching most rows of a large upload does not build a second copy of the file on the
heap, and the rewrite runs off the event loop the way the sibling full-file validation does.
Both proxy-injected metadata keys are captured from the record and restored exactly, including
an explicit null, so a masked row keeps the tags that decide how it is attributed.
A record is dropped only when a guardrail judged its content. `GuardrailRaisedException` now
carries `blocked_content` for that, because half its raise sites in the repo signal an
unreachable or unparseable backend under a fail-closed policy, and treating those as blocks
would turn "refuse this request" into "drop this record and submit the rest". The default is
off, so a raise that does not say what it means aborts the upload instead of silently
shrinking the file.
* fix(proxy): only drop a batch record on a verdict the guardrail actually reached
A guardrail that reports a technical failure as an HTTPException carrying a block status was
read as a content block, so an unreachable backend under a fail-closed policy quietly shrank the
file instead of failing the upload. Two in-tree integrations do exactly that, and one of them
defaults to fail-closed, so the broken configuration was the default one. Such an exception is
raised `from` the underlying error, which is a deliberate statement that something else caused
it, and no content verdict in the repo is raised that way, so the chain now settles it. Implicit
context is left alone, since a block raised inside an unrelated `except` would read as a failure.
Two annotation errors in the same family: the one GuardrailRaisedException subclass in tree never
opted into blocked_content, so a real block took the whole upload down with it, and straiker's
block helper is reached both from its verdict and from its fail-closed handler, so it claimed a
verdict for an outage. The helper now takes the flag from its caller.
A record could also opt itself out of the chain. Guardrail selection reads a body-level
`guardrails` key ahead of the proxy-injected list, and online that key can only add to the key
and team selection, never replace it, so a batch record naming an empty list skipped every
guardrail that was not default_on and was still reported as scanned. Every injected key is now
stripped before dispatch and restored afterwards.
A guardrail that reroutes a record to another model is honoured on the online path by rewriting
the model, which the scan read as a rewrite and submitted in the same file, sending content to
the provider the reroute existed to avoid. Every record of a batch file goes to one provider, so
the upload is refused instead, naming the line.
The scan spool is closed on the paths that never read it back.
* fix(proxy): give the scan the metadata bag guardrails actually read, and close its spools
The narrowed request metadata was installed under `litellm_metadata` only, but a record is
scanned as the chat request it describes, and the guardrails that pick a policy from a request
header read `metadata` instead. Noma choosing an application and Aim choosing a user both look
there, so the header allowlist added for them did not reach either one and a batch record was
still evaluated under the fallback policy. The scan metadata now goes into both bags, which are
both stripped and restored, so neither survives into the record that ships.
The scan spool was closed on the paths that abort, which are exactly the paths where it is
empty, and left open on the one path where it holds the rewritten records. Nothing closed the
rewrite output either, where before this feature the uploaded handle belonged to Starlette. The
upload now owns both and closes them however it exits.
* fix(proxy): register the scan spool before the rewrite can fail
The scan spool was added to the request's cleanup list only after the rewrite returned, so a
rewrite that raised, which for a spilled file can be as ordinary as the disk filling up, jumped
to the handler with the list still empty and left the scan's own handle open. The rewrite also
left its half-written output behind on that path, since nothing owns that handle until it is
returned. Both now close.
A terminal HTTP failure partway through chunking now logs the summed usage
and cost of the ApplyGuardrail calls AWS already billed, mirroring the
blocked-chunk path.
A blocked chunk now logs the summed usage and cost of every ApplyGuardrail
call AWS billed for the logical request, not just the blocking chunk.
Client-supplied metadata.standard_logging_guardrail_information is stripped
at the proxy boundary so callers cannot forge (even negative) guardrail
cost into spend, and guardrail_information_cost ignores negative or
non-finite entry costs as defense in depth. The cost map schema test now
allows guardrail_cost_per_unit and the guardrail mode.
A model with a long time-to-first-token leaves the proxy's response completely
idle, so any hop with an idle read timeout (AWS ALB and nginx both default to
60s) drops a connection that is perfectly healthy and would have delivered its
tokens shortly after.
The keepalive engines LiteLLM already ships wrap the response object, so they
fill a gap once the upstream has answered and then gone quiet. They cannot fill
the gap before it answers at all, and that is where the whole wait is spent:
measured against api.openai.com/v1/chat/completions with gpt-5.6 at
reasoning_effort high, the response headers and the first body byte both arrive
at 37.90s. Nothing has entered the ASGI response phase by then.
The upstream call is now raced against the keepalive interval, and when it
loses, the SSE response is opened immediately and ": ping" comments, which every
conformant SSE client ignores, fill the wire until the real response is ready to
be replayed onto it. One seam per funnel: base_process_llm_request covers every
native route, create_pass_through_route covers every passthrough route.
Committing the status line that early is the cost. A failure discovered after
the first ping reaches the client as an SSE error frame under a 200 rather than
as an HTTP error status, and LiteLLM's own x-litellm-* response headers are not
yet known. keepalive_ping_has_fired already documents the same trade-off for the
existing engines. Both are why this stays off until an operator sets
litellm_settings.sse_keepalive_ping_interval_seconds.
Separately, the passthrough relay reached neither engine even for mid-stream
gaps, which is the shape of #32491 and #24929, so the relayed bytes get the same
treatment, gated on the upstream declaring text/event-stream and only emitted
between complete frames so a binary transport (AWS event streams on /bedrock)
and a stall halfway through a frame are both left alone.
Fixes#34819
Price ApplyGuardrail usage units recorded by PR #37225 with a new
bedrock/guardrails entry in the model cost map (regional override via
bedrock/{region}/guardrails), add the per-request guardrail_cost to the
standard logging payload's response_cost and CostBreakdown, surface it in
the x-litellm-response-cost header, and bill blocked requests through the
failure hook so key and team budgets see what AWS bills
