Skip to content

fix(e2e): exclude the master key from transport repr - #32297

Open
mubashir1osmani wants to merge 1 commit into
litellm_internal_stagingfrom
litellm_e2e_transport_repr_masterkey
Open

fix(e2e): exclude the master key from transport repr#32297
mubashir1osmani wants to merge 1 commit into
litellm_internal_stagingfrom
litellm_e2e_transport_repr_masterkey

Conversation

@mubashir1osmani

Copy link
Copy Markdown
Collaborator

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

This commit was originally on the #32261 branch; that PR was closed after #32267 carried its other commits into staging, so this fix never landed. Cherry-picked onto a fresh branch

Screenshots / Proof of Fix

Before the fix, any failed e2e test printed the fixture repr chain, master key included, into CI output; the 2026-07-06 stage run's Loki export contains the stage master key verbatim inside the pytest failure blocks (fixture line: client = SpendClient(gateway=Gateway(transport=SplitTransport(data=HttpTransport(base_url=..., master_key='sk-640f...')

After the fix the canary test pins the repr chain:

$ uv run pytest tests/e2e/test_transport.py -q
18 passed in 0.08s

Type

Bug Fix

Changes

pytest prints fixture values verbatim into failure output, so every failed e2e test embedded the proxy master key in CI logs and whatever aggregator they ship to. Exclude master_key from HttpTransport's repr with field(repr=False) and add a canary test asserting the key never appears in the repr of HttpTransport or SplitTransport. Since past log exports already carry the stage master key, rotating it is recommended independently of this fix

pytest prints fixture values into failure output, so every failed e2e test was
embedding the proxy master key in CI logs and whatever aggregator they ship to;
the 2026-07-06 stage run's Loki export contains it verbatim. Exclude master_key
from HttpTransport's repr and pin the whole fixture repr chain with a canary
test. The stage master key should be rotated since past exports already carry
it
@mubashir1osmani

Copy link
Copy Markdown
Collaborator Author

@greptileai

@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a credential exposure issue in the e2e test layer by excluding master_key from HttpTransport's dataclass repr using field(repr=False), and adds a regression canary test to verify the key never appears in the repr of HttpTransport or SplitTransport.

  • tests/e2e/transport.py: One-line change to HttpTransportmaster_key: str becomes master_key: str = field(repr=False). This suppresses the field from the auto-generated __repr__, which SplitTransport's repr also delegates to when rendering its HttpTransport fields, so the protection cascades correctly.
  • tests/e2e/test_transport.py: New test test_transport_repr_never_leaks_the_master_key constructs both transport types with a known sentinel key and asserts the sentinel is absent from both reprs. The fix and the canary are appropriately paired.

Confidence Score: 5/5

Safe to merge — the change is a one-line suppression of a single dataclass field from repr, with no effect on runtime behavior, and the new canary test guards against regression.

Both changed files live entirely in the e2e test layer and have no production code impact. The field(repr=False) approach is the standard Python idiom for this purpose, and the cascading behavior through SplitTransport is correctly validated by the paired test. No logic, schema, or API surface is modified.

No files require special attention.

Important Files Changed

Filename Overview
tests/e2e/transport.py Adds field(repr=False) to master_key in HttpTransport to suppress it from the default dataclass repr, preventing credential exposure in pytest failure output.
tests/e2e/test_transport.py Adds test_transport_repr_never_leaks_the_master_key canary test; imports HttpTransport and SplitTransport and asserts that a known secret string does not appear in repr() of either class.

Reviews (1): Last reviewed commit: "fix(e2e): exclude the master key from tr..." | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds field(repr=False) to the master_key field in HttpTransport, preventing it from appearing in the dataclass's auto-generated __repr__. A canary test is added to assert the key never surfaces in either HttpTransport or SplitTransport repr output.

  • tests/e2e/transport.py: master_key now carries field(repr=False) alongside a docstring explaining the rationale; SplitTransport.__repr__ is safe by transitivity since its fields are HttpTransport instances whose repr already excludes the key.
  • tests/e2e/test_transport.py: New test test_transport_repr_never_leaks_the_master_key constructs both transport types with a sentinel key value and asserts neither repr contains it, pinning the fix against regressions.

Confidence Score: 5/5

Safe to merge — the change is confined to the e2e test infrastructure and has no effect on production code paths.

The fix is a one-line dataclass annotation on a test-only fixture file. The canary test correctly covers both the direct HttpTransport repr and the nested SplitTransport repr, so regressions would be caught immediately. No production modules are touched.

No files require special attention.

Important Files Changed

Filename Overview
tests/e2e/transport.py Adds field(repr=False) to master_key in HttpTransport to suppress it from the auto-generated __repr__, preventing key leakage in pytest failure output. Change is minimal and correct.
tests/e2e/test_transport.py Adds a canary test that asserts the master key string is absent from both HttpTransport.__repr__ and SplitTransport.__repr__ (which delegates to its nested HttpTransport instances). Correctly pins the intended behavior.

Reviews (1): Last reviewed commit: "fix(e2e): exclude the master key from tr..." | Re-trigger Greptile

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

1 participant