Skip to content

[Fix] Tests - Proxy: Isolate master_key/prisma_client module globals between tests#26362

Open
yuneng-berri wants to merge 6 commits intolitellm_internal_stagingfrom
litellm_fix_proxy_test_master_key_leak
Open

[Fix] Tests - Proxy: Isolate master_key/prisma_client module globals between tests#26362
yuneng-berri wants to merge 6 commits intolitellm_internal_stagingfrom
litellm_fix_proxy_test_master_key_leak

Conversation

@yuneng-berri
Copy link
Copy Markdown
Collaborator

Relevant issues

Summary

Failure Path (Before Fix)

Several sibling proxy tests mutate module-level globals on litellm.proxy.proxy_server (master_key, prisma_client) using raw setattr/direct assignment instead of monkeypatch. The values leak across tests within the same pytest-xdist worker. When a leaked non-None master_key is present, the auth short-circuit in user_api_key_auth (which accepts any bearer token when master_key is None) flips, and unrelated tests that pass Authorization: Bearer sk-test start returning 401 instead of 200.

Observed flakes in CI:

  • test_ui_view_session_spend_logs_pagination401 != 200
  • test_ui_view_spend_logs_date_range_filter401 != 200
  • test_ui_view_spend_logs_unauthorized400 != 401/403 (inverse case: test expected master_key to be set)

Fix

  1. Replace raw setattr(litellm.proxy.proxy_server, ...) / direct attribute assignment with monkeypatch.setattr in the two offending files:
    • tests/test_litellm/proxy/anthropic_endpoints/test_claude_code_marketplace.py (consolidated into one autouse fixture)
    • tests/test_litellm/proxy/realtime_endpoints/test_realtime_webrtc_endpoints.py
  2. Add an autouse fixture in tests/test_litellm/proxy/conftest.py that snapshots and restores master_key and prisma_client around every proxy test, so future raw mutations can't leak either.

Testing

  • Ran the originally-failing tests in isolation: pass.
  • Ran the leaky files interleaved with the spend tests (both orderings, xdist -n 4): all pass.
  • Temporarily reverted Option A and confirmed the conftest fixture alone catches the leak.
  • Broader proxy suite (tests/test_litellm/proxy/ under xdist) shows no new failures; the 1 failure and 3 errors present on this branch are also present on origin/main without these changes.

Type

✅ Test

mateo-berri and others added 4 commits April 23, 2026 14:07
Add gpt-5.5 entry with pricing from OpenAI flagship page:
input $5/1M, cached input $0.50/1M, output $30/1M, 272K context.
- Add gpt-5.5 to GPT5_MODELS parametrized list so both OpenAIGPT5Config
  and AzureOpenAIGPT5Config routing tests cover the new model.
- Add test_generic_cost_per_token_gpt55 verifying the new entry's
  cost-map values ($5/$0.50/$30 per 1M) and that generic_cost_per_token
  returns the expected prompt/completion costs.
feat: add gpt-5.5 to model cost map
…ests

Sibling tests were mutating litellm.proxy.proxy_server.master_key and
prisma_client with raw setattr. Values leaked across tests in the same
xdist worker, flipping the auth short-circuit in user_api_key_auth and
causing unrelated tests (e.g. test_ui_view_session_spend_logs_pagination)
to return 401 instead of 200.

Replace raw setattr with monkeypatch in the two offending files and add
an autouse conftest fixture that snapshots/restores the known-leaky
module globals for every proxy test.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 23, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 3 committers have signed the CLA.

✅ yuneng-berri
❌ mateo-berri
❌ shin-berri
You have signed the CLA already but the status is still pending? Let us recheck it.

@veria-ai
Copy link
Copy Markdown

veria-ai Bot commented Apr 23, 2026

Low: Test isolation improvements

This PR refactors test fixtures to use monkeypatch for proper cleanup of module globals (master_key, prisma_client) between tests. All changes are in test files only, with no impact on production code.


Status: 0 open
Risk: 1/10

Posted by Veria AI · 2026-04-24T01:20:49.037Z

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 23, 2026

Greptile Summary

This PR fixes test-isolation flakes in the proxy suite by replacing raw setattr/direct attribute mutations of litellm.proxy.proxy_server module-level globals (master_key, prisma_client) with monkeypatch.setattr, and adds an autouse fixture in conftest.py as a second safety net that snapshots and restores those globals around every proxy test. The fix is well-targeted: it addresses the exact auth short-circuit that caused 401/400 flakes under xdist workers and doesn't change any production code.

Confidence Score: 5/5

Safe to merge — test-only change with no production code impact.

All changes are confined to test files. They improve state isolation without weakening any assertions or test coverage. The two-layer cleanup (monkeypatch + conftest autouse) is redundant but harmless. No P0/P1 findings.

No files require special attention.

Important Files Changed

Filename Overview
tests/test_litellm/proxy/conftest.py Adds an autouse fixture that snapshots and restores master_key/prisma_client around every proxy test; clean safety-net implementation using sentinel for attribute-absence detection.
tests/test_litellm/proxy/anthropic_endpoints/test_claude_code_marketplace.py Consolidates seven per-test raw setattr calls into a single autouse monkeypatch fixture; logic and assertions are unchanged.
tests/test_litellm/proxy/realtime_endpoints/test_realtime_webrtc_endpoints.py Replaces manual try/finally teardown pattern in proxy_app fixture with monkeypatch.setattr; also removes a stray raw mutation in test_realtime_calls_success_with_valid_encrypted_token. Fixture correctly switches from yield to return since monkeypatch handles cleanup.

Sequence Diagram

sequenceDiagram
    participant xdist as xdist Worker
    participant conftest as conftest.py _isolate_proxy_module_globals
    participant mp as monkeypatch (per-test)
    participant ps as proxy_server module globals

    Note over xdist: Test Setup
    xdist->>conftest: fixture setup (autouse)
    conftest->>ps: snapshot {master_key, prisma_client}
    xdist->>mp: fixture setup
    mp->>ps: setattr master_key = sk-1234

    Note over xdist: Test Runs
    xdist->>ps: test uses master_key

    Note over xdist: Teardown (reverse order)
    xdist->>mp: teardown
    mp->>ps: restore master_key to snapshot value
    xdist->>conftest: teardown
    conftest->>ps: restore master_key to original snapshot (double-safe)

    Note over xdist: Next test sees clean globals
Loading

Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/lit..." | Re-trigger Greptile

Comment on lines +17 to +38

There are two distinct families:

* **gpt-5-chat family** (``gpt-5-chat``, ``gpt-5-chat-latest``,
``gpt-5-chat-2025-08-07``, …) — regular chat models that support ``temperature``
and ``tool_choice`` but NOT ``reasoning_effort``. Must NOT be on the GPT-5
reasoning path.

* **Versioned chat models** (``gpt-5.1-chat``, ``gpt-5.2-chat``,
``gpt-5.3-chat``, …) — ARE GPT-5 reasoning models and must stay on the GPT-5
path.

The fix uses a prefix check (``startswith("gpt-5-chat")``) on the normalised model
name instead of a substring check, which correctly distinguishes the two families.
"""

import pytest

from litellm.llms.openai.chat.gpt_5_transformation import OpenAIGPT5Config
from litellm.llms.azure.chat.gpt_5_transformation import AzureOpenAIGPT5Config

# ---------------------------------------------------------------------------
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Inaccurate background docstring

The module-level docstring makes two claims that don't match reality:

  1. "the string 'gpt-5-chat' is a substring of 'gpt-5.3-chat'" — this is false. Python's in operator checks for a contiguous character sequence; "gpt-5-chat" is not a substring of "gpt-5.3-chat" because the .3 between 5 and -chat breaks the match.

  2. "The fix uses a prefix check (startswith("gpt-5-chat"))" — the actual implementation in gpt_5_transformation.py still uses "gpt-5-chat" not in model, not startswith.

The tests themselves are correct and will pass with the current implementation, but the background section misdescribes both the alleged bug and the alleged fix, which can mislead future readers about why these cases need to be tested.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@yuneng-berri yuneng-berri temporarily deployed to integration-postgres April 23, 2026 23:06 — with GitHub Actions Inactive
@yuneng-berri yuneng-berri temporarily deployed to integration-postgres April 23, 2026 23:06 — with GitHub Actions Inactive
@yuneng-berri yuneng-berri temporarily deployed to integration-postgres April 23, 2026 23:06 — with GitHub Actions Inactive
@yuneng-berri yuneng-berri temporarily deployed to integration-postgres April 23, 2026 23:06 — with GitHub Actions Inactive
@yuneng-berri yuneng-berri temporarily deployed to integration-postgres April 23, 2026 23:06 — with GitHub Actions Inactive
…itellm_fix_proxy_test_master_key_leak

# Conflicts:
#	tests/test_litellm/proxy/realtime_endpoints/test_realtime_webrtc_endpoints.py
@gitguardian
Copy link
Copy Markdown

gitguardian Bot commented Apr 24, 2026

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
31539530 Triggered Generic Password e68c60a .github/workflows/_test-unit-services-base.yml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@ryan-crabbe-berri ryan-crabbe-berri self-requested a review April 24, 2026 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants