fix: X litellm safer variable injection #26
Conversation
WalkthroughPatch-level version bumps across multiple packages. Tests update environment variable keys from LITELLM_* to X_LITELLM_. Core logic changes map and export env vars using X_LITELLM_ (and X_LITELLM_EMBEDDINGS_ for embeddings) instead of LITELLM_*. No public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test/Evaluator
participant BE as BaseEvaluator.set_model_envs
participant ENV as Process Env
participant LP as litellm_patch.patched_completion
participant L as _original_completion
T->>BE: Provide env with X_LITELLM_* keys
BE->>ENV: Export model envs (X_LITELLM_* + known vars)
note right of BE: Azure alias handling unchanged
T->>LP: Invoke completion(**kwargs)
LP->>LP: Map X_LITELLM_* -> kwargs (skip all-uppercase, exclude X_LITELLM_EMBEDDINGS_*)
LP->>L: Call original completion(**mapped kwargs)
L-->>T: Return result
rect rgb(235, 245, 255)
note over BE,LP: Changed: X_LITELLM_* prefix handling for completions
end
sequenceDiagram
autonumber
participant T as Test/Evaluator
participant LP as litellm_patch.patched_embedding
participant E as _original_embedding
T->>LP: Invoke embedding(**kwargs)
LP->>LP: Map X_LITELLM_EMBEDDINGS_* -> kwargs (skip all-uppercase)
LP->>E: Call original embedding(**mapped kwargs)
E-->>T: Return embedding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
evaluators/ragas/pyproject.toml (1)
3-3: Patch release OK; consider guarding against upstream breaking changes.You’re using ragas==0.2.9 and open-ended langchain deps (>=). To reduce churn, consider adding an upper bound (e.g., <0.2.x+1 for major/minor) if CI isn’t catching breaks.
langevals_core/langevals_core/base_evaluator.py (1)
247-251: Avoid global env mutation races; consider scoped env.Setting os.environ in init can leak across concurrent evaluators/tests. Prefer a scoped context around the actual LLM calls.
Example context manager (apply where calls happen):
from contextlib import contextmanager @contextmanager def temp_env(updates: dict[str, str]): old = {k: os.environ.get(k) for k in updates} try: os.environ.update({k: v for k, v in updates.items()}) yield finally: for k, v in old.items(): if v is None: os.environ.pop(k, None) else: os.environ[k] = vThen wrap the evaluation that depends on these vars with temp_env(self.env or {}).
evaluators/langevals/tests/test_llm_boolean.py (1)
83-85: Skip gracefully when ATLA_API_KEY is missing.Prevents flaky external calls on CI when the token isn’t set.
Add a guard at the start of the test:
import pytest if not os.getenv("ATLA_API_KEY"): pytest.skip("ATLA_API_KEY not set; skipping external ATLA test")evaluators/langevals/tests/test_llm_category.py (1)
92-94: Guard test when ATLA_API_KEY is absent.Avoids CI flakes due to 401s against external API.
Add the same pytest skip guard as suggested in test_llm_boolean.py.
evaluators/langevals/tests/test_llm_score.py (1)
81-83: Skip when ATLA_API_KEY is not set.Prevents external dependency failures on CI.
Add the same pytest skip guard as suggested in test_llm_boolean.py.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (10)
evaluators/azure/poetry.lockis excluded by!**/*.lockevaluators/example/poetry.lockis excluded by!**/*.lockevaluators/huggingface/poetry.lockis excluded by!**/*.lockevaluators/langevals/poetry.lockis excluded by!**/*.lockevaluators/legacy/poetry.lockis excluded by!**/*.lockevaluators/lingua/poetry.lockis excluded by!**/*.lockevaluators/openai/poetry.lockis excluded by!**/*.lockevaluators/presidio/poetry.lockis excluded by!**/*.lockevaluators/ragas/poetry.lockis excluded by!**/*.lockpoetry.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
evaluators/azure/pyproject.toml(1 hunks)evaluators/example/pyproject.toml(1 hunks)evaluators/huggingface/pyproject.toml(1 hunks)evaluators/langevals/pyproject.toml(1 hunks)evaluators/langevals/tests/test_llm_boolean.py(1 hunks)evaluators/langevals/tests/test_llm_category.py(1 hunks)evaluators/langevals/tests/test_llm_score.py(1 hunks)evaluators/legacy/pyproject.toml(1 hunks)evaluators/lingua/pyproject.toml(1 hunks)evaluators/openai/pyproject.toml(1 hunks)evaluators/presidio/pyproject.toml(1 hunks)evaluators/ragas/pyproject.toml(1 hunks)langevals_core/langevals_core/base_evaluator.py(1 hunks)langevals_core/langevals_core/litellm_patch.py(2 hunks)langevals_core/pyproject.toml(1 hunks)
🔇 Additional comments (14)
evaluators/example/pyproject.toml (1)
3-3: Version bump looks fine.No functional changes here. Ensure lockfile is regenerated and published artifacts reference the updated core.
evaluators/huggingface/pyproject.toml (1)
3-3: Version bump LGTM; keep an eye on httpx compatibility.No code changes; deps unchanged. Verify your httpx and litellm versions remain compatible with the patched env-var behavior.
evaluators/azure/pyproject.toml (1)
3-3: Patch bump approved.No functional diffs; dependency set unchanged. Ship with updated lockfile.
evaluators/presidio/pyproject.toml (1)
3-3: Version bump looks good.No functional impact here.
evaluators/lingua/pyproject.toml (1)
3-3: Version bump approved.Consistent with the repo-wide patch releases.
evaluators/langevals/pyproject.toml (1)
3-3: Version increment OK.Given the env var prefix change elsewhere, ensure release notes mention the X_LITELLM_* migration.
evaluators/openai/pyproject.toml (1)
3-3: Version bump LGTM.No dependency changes tied to this file.
langevals_core/langevals_core/base_evaluator.py (2)
247-251: Aligns auto-export with X_LITELLM_ — good.*Matches the safer-prefix strategy and keeps provider vars working.
247-251: No bareLITELLM_*references detected. Only intentionalX_LITELLM_*usages remain inlitellm_patch.pyand its tests.evaluators/legacy/pyproject.toml (1)
3-3: Version bump looks good.Patch-level update aligned with the rest of the PR.
evaluators/langevals/tests/test_llm_boolean.py (1)
83-85: Env key rename LGTM.Matches new X_LITELLM_* mapping.
evaluators/langevals/tests/test_llm_category.py (1)
92-94: Env key rename LGTM.Consistent with litellm patch.
evaluators/langevals/tests/test_llm_score.py (1)
81-83: Env key rename LGTM.Aligned with X_LITELLM_* convention.
langevals_core/langevals_core/litellm_patch.py (1)
32-40: No lingering LITELLM_ usages detected. Ripgrep search for unprefixedLITELLM_returned no results, confirming all legacy prefixes have been removed.
| key.startswith("X_LITELLM_") | ||
| and not key.startswith("X_LITELLM_EMBEDDINGS_") | ||
| ): | ||
| kwargs[key.replace("LITELLM_", "")] = value | ||
| replaced_key = key.replace("X_LITELLM_", "") | ||
| # check if key is all uppercase, likely not a litellm key and got here by accident | ||
| if replaced_key.isupper(): | ||
| continue | ||
| kwargs[replaced_key] = value | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden env->kwargs mapping: don’t override reserved/caller args; ignore empty values.
As written, X_LITELLM_* can override reserved keys like drop_params, cache, model, or any caller-provided kwarg, weakening the “safer” intent. Add guards.
Apply this diff:
if (
key.startswith("X_LITELLM_")
and not key.startswith("X_LITELLM_EMBEDDINGS_")
):
replaced_key = key.replace("X_LITELLM_", "")
# check if key is all uppercase, likely not a litellm key and got here by accident
if replaced_key.isupper():
continue
+ # don't let env override reserved or caller-provided args; ignore empty values
+ if replaced_key in {"drop_params", "cache", "model", "vertex_credentials"}:
+ continue
+ if replaced_key in kwargs:
+ continue
+ if value in ("", None):
+ continue
kwargs[replaced_key] = value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| key.startswith("X_LITELLM_") | |
| and not key.startswith("X_LITELLM_EMBEDDINGS_") | |
| ): | |
| kwargs[key.replace("LITELLM_", "")] = value | |
| replaced_key = key.replace("X_LITELLM_", "") | |
| # check if key is all uppercase, likely not a litellm key and got here by accident | |
| if replaced_key.isupper(): | |
| continue | |
| kwargs[replaced_key] = value | |
| if ( | |
| key.startswith("X_LITELLM_") | |
| and not key.startswith("X_LITELLM_EMBEDDINGS_") | |
| ): | |
| replaced_key = key.replace("X_LITELLM_", "") | |
| # check if key is all uppercase, likely not a litellm key and got here by accident | |
| if replaced_key.isupper(): | |
| continue | |
| # don't let env override reserved or caller-provided args; ignore empty values | |
| if replaced_key in {"drop_params", "cache", "model", "vertex_credentials"}: | |
| continue | |
| if replaced_key in kwargs: | |
| continue | |
| if value in ("", None): | |
| continue | |
| kwargs[replaced_key] = value |
🤖 Prompt for AI Agents
In langevals_core/langevals_core/litellm_patch.py around lines 32-40, the
env->kwargs mapping currently replaces keys from X_LITELLM_* even if they
collide with reserved names or with kwargs already passed, and it also accepts
empty values; update the loop to 1) skip any replaced_key that is in a reserved
set (e.g., {"drop_params","cache","model"} plus any other internal names) or
already present in kwargs to avoid overriding caller-provided args, 2) ignore
values that are None or empty strings (treat them as absent), and 3) continue to
preserve the existing uppercase check; if a key is skipped, do not set it in
kwargs. Ensure the check order is: compute replaced_key, skip if
replaced_key.isupper(), skip if value is falsy/empty, skip if replaced_key in
reserved set or replaced_key in kwargs, otherwise assign kwargs[replaced_key] =
value.
| if key.startswith("X_LITELLM_EMBEDDINGS_"): | ||
| replaced_key = key.replace("X_LITELLM_EMBEDDINGS_", "") | ||
| # check if key is all uppercase, likely not a litellm key and got here by accident | ||
| if replaced_key.isupper(): | ||
| continue | ||
| kwargs[replaced_key] = value | ||
| return _original_embedding(*args, **kwargs) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same hardening for embeddings env mapping.
Mirror the reserved/empty/override guards.
Apply this diff:
for key, value in os.environ.items():
- if key.startswith("X_LITELLM_EMBEDDINGS_"):
- replaced_key = key.replace("X_LITELLM_EMBEDDINGS_", "")
- # check if key is all uppercase, likely not a litellm key and got here by accident
- if replaced_key.isupper():
- continue
- kwargs[replaced_key] = value
+ if key.startswith("X_LITELLM_EMBEDDINGS_"):
+ replaced_key = key.replace("X_LITELLM_EMBEDDINGS_", "")
+ # check if key is all uppercase, likely not a litellm key and got here by accident
+ if replaced_key.isupper():
+ continue
+ # don't let env override reserved or caller-provided args; ignore empty values
+ if replaced_key in {"drop_params", "model"}:
+ continue
+ if replaced_key in kwargs:
+ continue
+ if value in ("", None):
+ continue
+ kwargs[replaced_key] = value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if key.startswith("X_LITELLM_EMBEDDINGS_"): | |
| replaced_key = key.replace("X_LITELLM_EMBEDDINGS_", "") | |
| # check if key is all uppercase, likely not a litellm key and got here by accident | |
| if replaced_key.isupper(): | |
| continue | |
| kwargs[replaced_key] = value | |
| return _original_embedding(*args, **kwargs) | |
| for key, value in os.environ.items(): | |
| - if key.startswith("X_LITELLM_EMBEDDINGS_"): | |
| - replaced_key = key.replace("X_LITELLM_EMBEDDINGS_", "") | |
| - # check if key is all uppercase, likely not a litellm key and got here by accident | |
| - if replaced_key.isupper(): | |
| - continue | |
| if key.startswith("X_LITELLM_EMBEDDINGS_"): | |
| replaced_key = key.replace("X_LITELLM_EMBEDDINGS_", "") | |
| # check if key is all uppercase, likely not a litellm key and got here by accident | |
| if replaced_key.isupper(): | |
| continue | |
| # don't let env override reserved or caller-provided args; ignore empty values | |
| if replaced_key in {"drop_params", "model"}: | |
| continue | |
| if replaced_key in kwargs: | |
| continue | |
| if value in ("", None): | |
| continue | |
| kwargs[replaced_key] = value | |
| return _original_embedding(*args, **kwargs) |
🤖 Prompt for AI Agents
In langevals_core/langevals_core/litellm_patch.py around lines 54 to 60, the
embeddings env mapping block needs the same hardening as the other env mappings:
add guards to skip processing when replaced_key is empty, skip when
replaced_key.isupper() (already present), skip when replaced_key is in the
module's reserved keys list (use the existing RESERVED_ENV or RESERVED_KEYS
constant used elsewhere in this file), and skip if replaced_key already exists
in kwargs to avoid overriding explicit values; only after these checks set
kwargs[replaced_key] = value and then return _original_embedding(*args,
**kwargs).
| @@ -1,6 +1,6 @@ | |||
| [tool.poetry] | |||
| name = "langevals-core" | |||
| version = "0.1.14" | |||
| version = "0.1.15" | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Patch bump OK; verify that the env-var prefix change isn’t a breaking release.
Given the switch to X_LITELLM_* in core logic, confirm no consumers depend on LITELLM_* from process env. If that was externally observable, consider a minor (0.1.x -> 0.2.0) instead of patch.
Run to surface leftover LITELLM_* usage and align deps:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Leftover LITELLM_* references (should be empty):"
rg -nP -C2 '\bLITELLM_' -g '!**/dist/**' -g '!**/build/**' -g '!**/.venv/**' || true
echo
echo "New X_LITELLM_* keys usage (expect hits):"
rg -nP -C1 '\bX_LITELLM_(api_key|api_base|EMBEDDINGS_)'
echo
echo "litellm constraints across packages (ensure consistency):"
rg -nP '^\s*litellm\s*=\s*"[^^][^"]*|^\s*litellm\s*=\s*\^" -g '**/pyproject.toml' || trueLength of output: 2436
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "litellm dependency constraints across all pyproject.toml files (ensure consistency):"
rg -nP '^\s*litellm\s*=\s*(".*?"|\{.*?\})' -g '**/pyproject.toml' -g '!**/dist/**' -g '!**/build/**' -g '!**/.venv/**'Length of output: 696
Bump to 0.2.0 to reflect breaking env-var change
Verified no residual LITELLM_* references and consistent litellm deps, but switching to X_LITELLM_* is externally breaking. In langevals_core/pyproject.toml (line 3), change
-version = "0.1.15"
+version = "0.2.0"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| version = "0.1.15" | |
| ++ b/langevals_core/pyproject.toml | |
| @@ lines 1-5 | |
| version = "0.2.0" |
🤖 Prompt for AI Agents
In langevals_core/pyproject.toml around line 3, update the package version from
"0.1.15" to "0.2.0" to reflect the breaking change of switching environment
variable prefixes (LITELLM_* → X_LITELLM_*); modify the version string
accordingly and verify that any release notes or changelogs and downstream
dependency references are updated to indicate the breaking env-var change.
Summary by CodeRabbit