A parenthesised VALUES list ended the search for an insert's row source
only when no group followed it, so a RETURNING or an ON CONFLICT DO
UPDATE carrying a subquery was read as the rows the insert copies. A
writable CTE bounded by its own VALUES list was handed the query the
statement ends with for the same reason: the WITH branch read the whole
statement rather than the part holding the insert.
A CREATE FUNCTION or CREATE PROCEDURE body was scanned as if it ran at
boot, but defining a routine only stores it. The body is now read when
the same migration names the routine somewhere else, so a migration that
defines a backfill and then runs it is still caught, and one whose name
needed quoting is read either way since quoting is blanked at the call
sites too.
main() had no test, so neither its exit codes nor the branch the CI gate
reads were pinned; a mutant returning 0 on a violation passed the whole
suite. Its four outcomes now have tests, along with both directions of
each fix above.
Taking the last group at the statement's outermost level assumed the row
source was written there, and an insert is allowed to carry more after it:
`(SELECT ...) ON CONFLICT ("id") DO NOTHING` ends on the conflict target
and `... RETURNING ("id")` on the returning list, so the query supplying
the rows was never reached and a full table copy passed the gate.
Each group is now read on its own terms and the first to name a row source
is the answer, since the others are the column list and the clauses an
insert may carry, none of which names one.
An `INSERT` whose `VALUES` list holds a scalar subquery was reported as a
rewrite whenever that list was not the plain top-level one: joined to
another term by `UNION`, `INTERSECT` or `EXCEPT`, or written inside
parentheses, which Postgres accepts. Both shapes insert a fixed handful of
rows, so the gate was rejecting migrations that do nothing wrong.
A set operation is now split into its terms and each is read on its own,
since the insert is a rewrite when any one term is a query. A row source
kept in parentheses is read on its own terms too. The operators are found
outside every parenthesis, so a set operation written inside a `VALUES`
list does not cut the list in half.
A `JOIN ... USING` inside a subquery that helps build an EXECUTE's command
was taken for the start of its bind values, so anything written after it
went unscanned and a rewrite there was never reported. Only a `USING` with
the parentheses closed can be the bind-values clause.
A `FOR ... LOOP` header carries no semicolon of its own, so the first
statement of the loop body is written into the same semicolon-delimited
run. Reading the pair as one statement let the header's row source stand
in as the keyword for both, which hid whatever the loop repeats: a plain
`UPDATE` in a query-driven loop went unreported, and so did an `EXECUTE`
of one. That is the shape a row-by-row backfill takes, and it is the
shape this gate exists to stop.
Reading every operator let a comparison beside an assignment look like one.
`ok := n = 1 AND stmt = '<dml>'` registered `stmt` as written, which collided
with the `EXECUTE stmt` further down and flagged a block that rewrites nothing.
A statement holds one assignment at most, so the search now stops at the first
operator that reads as one: everything after it is the expression being
assigned, where an `=` only ever compares. Nine shapes were flagged this way,
a cast, a `coalesce`, a `format`, a named-argument arrow and the rest, and all
of them are valid PL/pgSQL that leaves the table untouched.
`INTO` and `USING` no longer count as names an `EXECUTE` runs. Masking blanks a
literal in place, so `EXECUTE '<sql>' INTO n` left `INTO` looking like the name
being run, and an ordinary query reaching the same word collided with it. The
docstring claiming that collision was impossible was wrong, and both words are
now dropped instead.
A loop is a fourth way a literal reaches a variable. `FOR stmt IN SELECT
'<dml>' LOOP EXECUTE stmt` empties the table and the gate passed it, so the
target of a `FOR` or a `FOREACH` is read as assigned too.
Reading each statement once rather than once per operator also drops the cost
of a statement with thousands of them from seconds to milliseconds.
A bare = was found with partition, so a comparison earlier on the line took
the one slot and the assignment after it went unread. INTO targets and the
name an EXECUTE runs are also allowed to sit on the next line now.
* 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
Scanning a statement's own literals for SQL only makes sense when the name
before INTO is a variable the body later executes. INSERT INTO names a table
there, so an insert into a table sharing a variable's name was flagged for
whatever its column values happened to spell. An INSERT that really does
assign reaches INTO through RETURNING, which the preceding word separates.
Also names the scope boundary in the module docstring: the ban is on
row-rewriting DML, not on everything whose cost scales with table size.
A parenthesised query term joined to a top-level VALUES list sat behind
strip_parens, so an insert reading `VALUES (1) UNION ALL (SELECT ...)` copied a
whole table past the gate. A VALUES list now bounds an insert only while no set
operation sits beside it at that same level.
PL/pgSQL also parks dynamic SQL in a variable through a query's INTO and through
the bare `=` it takes as the assignment operator, and assigned_names read
neither, so a rewrite handed to a later EXECUTE went unseen. A bare `=` counts
only where the words ahead of it make it an assignment rather than a test.
Postgres takes the row source parenthesised, so `INSERT INTO "t" ("a") (SELECT
...)` copies a whole table at boot. Reading only the unparenthesised text let it
through: 777eb8af10 caught it, then e7dea842c3 traded it away to stop a VALUES
list joined to a query by a set operation from bounding nothing.
Read the top level first so set operations still count, then fall back to the
whole statement when no top-level VALUES bounds the insert. `TABLE t` is a row
source as much as a `SELECT` is, and it was passing too
Four more ruff rules for code the test suite runs but never checks. F601 is the
one that paid: the duplicate key it flagged in a get_form_data fixture was the
mock reproducing the production bug fixed in the previous commit.
B025 removed two unreachable handlers, one of them a pytest.skip shadowed by an
earlier `pass`, so an upstream Vertex flake reported green having asserted
nothing. F632 turned an `is ""` identity check, which passes only on CPython
interning, into the `== ""` it meant. B023 fixed three closures over loop
variables, all latent today but one iteration-order change away from checking the
last case N times.
A dollar-quoted payload is read as its own region rather than as a handed-off
string, so the marker belongs on the rewrite inside it. Pin that placement, and
pin that a marker on a DO block header never covers the block's body.
EXPLAIN ANALYZE runs the statement it wraps rather than only planning it, but
ANALYZE sits in the keyword set, so it stood in for the keyword underneath and a
rewrite left under one reached boot unflagged.
DO takes its body as a string literal, and dollar quoting is a convenience
rather than a requirement. A migration spelling the body in single quotes
got its rewrite through untouched, since nothing was reading that literal
as SQL. It is ordinary syntax rather than an attempt to hide anything, so
the miss was reachable by accident.
The module docstring now also records where concatenated dynamic SQL stops
being readable, which is a keyword split across fragments that do not hold
it. Every fragment is scanned, so the shapes people actually write are all
still caught.
A marker on an EXECUTE now covers the SQL that EXECUTE runs, so it goes
where the migration reads rather than inside the string. A literal whose
first line sat below its EXECUTE was missing the marker entirely, and the
documented placement failed CI.
A literal assigned with := counts as SQL only when an EXECUTE in the same
body runs that variable by name. An error message naming a DELETE the
application handles is text, and the only way to silence it before was a
marker claiming a bounded data migration that was not there at all.
scan() recursed into a dollar-quoted body with the sliced text but kept absolute
offsets, so line_of counted newlines in the slice against a position past its end.
Any DO $$ block below the first line reported a wrong line, which also misaligned
the -- data-migration-ok: markers: an unrelated marker earlier in the file could
exempt a rewrite inside a block, and a marker sitting right above one failed to.
Line numbers now always count against the whole migration text.
EXECUTE was treated as harmless while its quoted SQL was masked, so a rewrite
handed over as a string walked through the gate. The literal an EXECUTE runs is
now scanned like a dollar-quoted body.
INSERT was classified by searching the whole statement for SELECT, so a bounded
INSERT ... VALUES holding a scalar subquery, or led by a helper CTE, was flagged
as INSERT ... SELECT. A top-level VALUES now bounds the insert, and a VALUES
buried in a subquery still does not.
Drops the doubled-quote branch in skip_quoted, which masked the same span
either way and so could not be covered, and orders the failure report before
the guidance text.
* test: run the 30 test files stranded in the second mirror
tests/litellm sat beside tests/test_litellm, which is the mirror the repo
convention names, and no job collected it. The allowlist called the directory
unresolved and assumed it was a duplicate. It is not: 30 of its 34 files have no
counterpart in the real mirror, so they are tests nobody has run since they were
written, not copies of tests that run elsewhere.
Moving them in is byte-identical, and it is what makes them run. Every one is
now claimed by a shard's test-path rather than by an allowlist entry, and the
216 tests they hold pass. Directories that needed to become packages did, since
several files are named test_transformation.py and pytest cannot import two of
those from non-package directories in one session.
Never running is why three assertions had drifted away from the code:
* nvidia.nemotron-super-3-120b max_output_tokens, 32000 -> 32768
* sambanova/MiniMax-M2.7 max_input_tokens, 204800 -> 196608
* the Vertex text-to-speech handler moved from data= to json=, so the test
reads the decoded body off the json kwarg instead of parsing the data one
The first two follow model_prices_and_context_window.json, which the catalog
sync keeps current; the third follows the handler. In all three the test was the
stale side.
The lint workflow ran test_no_hardcoded_secrets.py by path and now points at the
new one.
Four files stay behind. Each shares a filename with a live test whose contents
are disjoint from it, so landing those means merging test bodies, which is a
content review rather than a move. The allowlist entry now names those four and
records how many tests each would bring, in place of calling the whole
directory unresolved.
* fix(ci): keep the secret scan out of the mirror's conftest
The secret-scan job runs pytest under uv run --no-project, so its environment
holds pytest and nothing else. That worked while the file sat in tests/litellm,
which has no conftest, and broke the moment it moved into tests/test_litellm,
whose conftest imports litellm on collection: ModuleNotFoundError: No module
named 'dotenv', before a single test ran.
The file is a repo-wide static scan that imports only base64, os, re and pytest,
so it belongs with the other repo-wide checks in tests/code_coverage_tests,
which has no conftest, rather than in the package mirror. Installing the full
dependency set into a 15-second job to satisfy a conftest it does not use would
be the wrong trade.
Verified with the job's exact command:
uv run --no-project --with 'pytest==9.0.2' pytest \
tests/code_coverage_tests/test_no_hardcoded_secrets.py -q
1 passed in 0.47s
* feat(search): add Nimble as a search provider
Adds `NimbleSearchConfig` so `search_provider: nimble` works across the SDK,
the proxy /v1/search endpoint, the Search Tools dashboard, and spend tracking.
Nimble's /v2/search already uses the Perplexity unified spec's parameter names,
so the request transform is close to a pass-through. `search_domain_filter`
splits into include_domains/exclude_domains on the spec's `-` prefix, `country`
is upper-cased to the ISO form Nimble documents, and everything else is
forwarded so focus, search_depth, time_range and the rest stay reachable. On the
response side, snippet prefers `content` and falls back to `description`, and a
malformed body raises an attributed error rather than reporting an empty search.
Also tightens `BaseSearchConfig.get_supported_perplexity_optional_params` to
return `frozenset[str]` instead of a bare mutable `set`, which every caller
already treats as read-only.
* fix(search): surface Nimble error bodies instead of empty results
Greptile flagged that a null or absent `results` degraded to a successful empty
search. A search with no hits comes back as `"results": []`, verified against the
live API, so the field is now required and anything else raises the attributed
schema error the other malformed bodies already take.
Also unwraps Nimble's second error envelope. Collection failures return
`{"success", "task_id", "message"}` rather than the `{"detail"}` shape validation
errors use, and only the latter was being read.
Drops comments that restated the adjacent code.
* docs(search): drop the Nimble param list from the transform docstring
It restated the vendor's API reference, which the module docstring already links,
and would go stale the moment Nimble adds a focus mode.
The timeout contract check skipped a job whenever either budget came from
a `with:` value it could not parse, or from a matrix column no `include`
row supplied as a number. Both paths produced no pairs and no errors, so
the guard printed "invariants hold" for a caller whose budgets were never
compared at all. A caller reading `${{ matrix.timeout }}` off a mistyped
column while capping the job at 1 minute passed clean.
Unresolvable budgets now come back as the reason they could not be read
and are reported as violations, which is the whole point of a guard built
to catch checks that silently do not run. `Column` tags a matrix
reference so it stays distinguishable from that reason string, and the
report names only the columns that resolve nowhere, since a column every
row supplies is not what left the pair unchecked.
The timeout contract check resolved the test and job budgets
independently and compared every value against every other, so two
matrix-sourced columns were paired across different include rows. A
row-wise-valid matrix could be rejected on a pairing no shard actually
runs with. Budgets now resolve per include row, so each shard's test
budget is checked only against that same shard's job budget.
GitHub expressions have no arithmetic operators, so
`${{ inputs.timeout-minutes + inputs.setup-timeout-minutes }}` was not a value
but a startup failure. The proxy-db workflow died before creating any job on
both prior commits, which posts no check run at all: the entire suite stopped
running while the PR's checks stayed green.
Pass the job backstop in as `job-timeout-minutes` instead of computing it, and
size it as the test budget plus the 30 minutes of setup ceilings plus 5 minutes
of runner overhead the job clock charges but no step owns.
check_workflow_startup_safety.py makes this class of mistake visible before
merge, since CI cannot report it: it rejects arithmetic inside an expression
and checks every caller of the reusable workflow keeps a job budget large
enough that the deadline cannot preempt pytest inside its own budget.
The summed job deadline alone did not protect the test budget. Setup that
overran its allowance still ate into pytest's window, which is the same
failure this change set out to remove, just with more headroom.
Every step before pytest now carries its own ceiling, and their sum is the
`setup-timeout-minutes` default. Setup can no longer overrun into the test
budget without failing its own step first, and a slow setup step now reports
as a red step naming itself rather than a cancelled shard whose tests passed.
Model the workflow YAML the guard reads with Pydantic instead of bare dicts,
so the shapes it depends on are validated once at the boundary. A workflow
that does not parse is now reported as a finding rather than a traceback.
`prisma generate` runs `npm install prisma@<version>` whenever the
prisma-client-py binary cache directory has no CLI entrypoint, pulling ~85 MB
of query and schema engines over the network. Every workflow pointed
PRISMA_BINARY_CACHE_DIR at `${{ runner.temp }}/prisma-cache`, which GitHub
wipes and recreates per job, so that cache was empty on every job of every
run and the download was never avoidable.
The download is normally a few seconds and occasionally minutes. On one
proxy-db run it took 5m18s on a single shard against 3.8s on its eleven
siblings, which pushed the job past its 15 minute timeout and cancelled a
shard whose tests were at 99% and all passing.
Leave PRISMA_BINARY_CACHE_DIR unset so the binaries land in the
prisma-client-py default, which is already keyed by prisma and engine
version, and restore both that path and the @prisma/engines staging cache
through a shared composite action.
Job timeouts also counted setup against the test budget. `timeout-minutes`
now bounds the pytest step, with a separate allowance for checkout,
dependency install, and client generation, so slow setup shows up as a slow
job instead of a cancelled test run.
check_prisma_binary_cache.py guards all three invariants: no workflow
reintroduces the override, every job that generates the client restores the
cache, and the version the action greps out of uv.lock still resolves.
Guardrails silently skipped three surfaces on the Anthropic Messages
path, so an agent loop driven by /v1/messages ran unguarded:
- The Anthropic input translation never walked tool_result blocks, so
content returned by a local tool (a curl, a file read, an MCP call)
reached the model unscanned in both the string and list content
shapes, images inside a tool_result included.
- tool_permission only understood ModelResponse, so an Anthropic
non-streaming response or a raw SSE stream carrying tool_use blocks
passed through with no rule ever evaluated.
- ContentFilterGuardrail scanned inputs["texts"] but never
inputs["tool_calls"], so the arguments a model proposes for a tool
call went unchecked.
Tool call arguments are parsed as JSON before filtering so a MASK
action rewrites the value and leaves the payload valid JSON; non-JSON
arguments fall back to scanning the raw string. Denied tool_use blocks
are dropped from the Anthropic content array and replaced with a text
block, and stop_reason resets to end_turn when nothing tool-shaped
survives.
Guardrails could only see the MCP tool call request (pre_mcp_call /
during_mcp_call); the tool result went back to the client unscanned, so a tool
that returns sensitive data bypassed every configured guardrail.
Adds a `post_mcp_call` event hook that runs after the tool executes and routes
the result through the unified apply_guardrail seam, so a text guardrail (e.g.
presidio) can mask sensitive values in the tool output or reject the result
without any MCP-specific code of its own.
- MCPGuardrailTranslationHandler.process_output_response now extracts the tool
result's text content into GenericGuardrailAPIInputs["texts"], calls
apply_guardrail with input_type="response", and writes the returned text back
into the content list in place (the logging payload already references that
object, so a copy would leave the unmasked text in the spend log)
- ProxyLogging.post_mcp_call_hook dispatches guardrails that implement
apply_guardrail, gated on should_run_guardrail(post_mcp_call); guardrails
implementing async_post_mcp_tool_call_hook keep their existing dispatch and
are not run twice
- both MCP tool-call paths (mcp_server and the Responses API handler) now honor
the rewritten result, and the REST path no longer swallows a guardrail
rejection as a logging failure
- shared, duck-typed MCP content helpers live in mcp_server/utils.py next to
extract_mcp_tool_result_error_message
- documents that async_post_mcp_tool_call_hook's return value is discarded by
every call site, so that hook only takes effect by mutating in place
Adds get_external to e2e_http.py for absolute third-party GETs (no proxy base url or auth, same Result classification) and rewires fetch_agent_card through it, dropping the urllib.request escape hatch. Creates tests/code_coverage_tests/check_e2e_no_raw_requests.py, the checker tests/e2e/CLAUDE.md already referenced, and wires it into the code-quality workflow so raw HTTP client imports outside the transport fail CI; pre-existing uses (root conftest liveness probe, claude_code version resolver) are grandfathered and exception-type-only imports stay allowed.
* fix(model_armor): sanitize error details by default
Generated with AI
Co-Authored-By: Claude Code
* fix(model_armor): sanitize handler-raised HTTP errors and redact scanned content in guardrail logging
The async HTTP handler raises MaskedHTTPStatusError on any non-2xx via
raise_for_status, so the non-200 branch in make_model_armor_request never ran
against a live API and the raw upstream body reached callers and logs. Catch
the raised error and build the sanitized detail from the response status
Replace the empty-dict guardrail logging payload with field-level redaction of
the keys that echo scanned content (text, sanitizedText, findings) so guardrail
traces keep filter states and block reasons while scanned content stays out
Restore the upstream status code in the sanitized error detail, read guardrail
metadata from the same key the hooks write, and keep guardrail_status within
its typed literal values
* fix(model_armor): bound redactor recursion depth and allowlist it in the recursion detector
_redact_scanned_content walks provider JSON bounded by _REDACT_MAX_DEPTH=20 and
fails closed by returning the redaction sentinel at the cap
* fix(model_armor): honor fail_on_error for upstream API failures
API failures now raise a dedicated ModelArmorAPIError so hooks can tell them
apart from content-block HTTPExceptions; fail_on_error=False lets the request
proceed on a Model Armor outage again while fail-closed configs get the same
sanitized 400 as before
Also addresses review notes: sanitize_error_detail constructor annotation
matches the nullable config field, redaction is owned by the metadata write
sites so _process_response no longer re-applies it, and the request and
response debug log branches move into helpers
* test(model_armor): cover fail_on_error routing on during-call, post-call, streaming, and file-scan paths
* chore: remove accidentally committed pytest cache files
* fix(model_armor): keep sanitize_error_detail coerced across in-memory config reloads
update_in_memory_litellm_params assigns raw LitellmParams fields, so a hot
reloaded config carrying an explicit null would silently disable sanitization;
re-apply the only-explicit-False-opts-out coercion after the update
* fix(model_armor): redact matched malicious URIs and reuse the shared recursion depth constant
maliciousUriMatchedItems echoes the caller-supplied URL including path and
query, so it joins the scanned-content key set; the redactor depth cap now
comes from DEFAULT_MAX_RECURSE_DEPTH in litellm constants instead of a local
literal
* fix(model_armor): keep API failures out of the intervention trace status
Fail-closed upstream failures re-raise ModelArmorAPIError instead of
converting to HTTPException(400), so the shared guardrail logging keeps
recording them as guardrail_failed_to_respond while content blocks stay
guardrail_intervened. Callers see the same 500 shape as before this PR,
with the sanitized message
* chore(model_armor): drop explanatory comment per repository comment policy
---------
Co-authored-by: eugene-yao-zocdoc <eugene.yao@zocdoc.com>
* feat(guardrails): add pre_mcp_call support to Content Filter
* test(guardrails): cover canonical MCP key gate under pre_mcp_call mode
* fix(guardrails): scan MCP arguments per value and gate mixed-mode scans by call type
* fix(guardrails): cap MCP argument scan depth and register the walker with the recursion detector
* test(guardrails): update LIT-4226 UI settings tests for content filter pre_mcp_call support
* fix(guardrails): use builtin generics in MCP scan annotations to satisfy strict-rule budget
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
apply_json_merge_patch recurses into nested objects, which the repo's recursive_detector code-quality check flags because unbounded recursion over caller-supplied JSON has caused CPU/stack issues before. Cap the recursion at a depth far above any realistic team-metadata shape and reject deeper patches with a ValueError so a pathologically nested body fails closed instead of overflowing the stack, then register the function in the detector's ignore list alongside the other depth-bounded JSON walkers
Team-level callback_vars (e.g. langsmith_api_key) get spread into
data["metadata"] as four aliases (user_api_key_metadata,
user_api_key_team_metadata, user_api_key_auth_metadata,
user_api_key_auth). When a guardrail hook echoes that metadata into
its guardrail_response, the plaintext credential landed five times
inside LiteLLM_SpendLogs.metadata.standard_logging_guardrail_information[i].guardrail_response
and every downstream sink that reads it (OTel via emit_guardrail_span,
Langfuse, custom loggers).
Add a purpose-built payload walker (mask_credentials_in_payload) that
only masks strings under sensitive-named keys and preserves every
other value (None, ints, floats, bools, tuples, typed objects) verbatim.
The walker reuses SensitiveDataMasker.is_sensitive_key so the pattern
list stays in one place, and unwraps Pydantic models via model_dump()
so nested UserAPIKeyAuth values reached by the walk get scanned as
plain dicts (they are JSON-serialized downstream anyway).
Apply the walker at add_standard_logging_guardrail_information_to_request_data
after the existing secret_fields pop and match/regex redaction, so
every downstream sink sees masked values from a single seam.
