Skip to content

test(guardrails): stop the onyx and prompt security tests leaking env vars on failure - #37828

Merged
yuneng-berri merged 3 commits into
litellm_internal_stagingfrom
litellm_onyx_env_leak
Aug 22, 2026
Merged

test(guardrails): stop the onyx and prompt security tests leaking env vars on failure#37828
yuneng-berri merged 3 commits into
litellm_internal_stagingfrom
litellm_onyx_env_leak

Conversation

@yuneng-berri

@yuneng-berri yuneng-berri commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Five guardrail test files write straight to os.environ
  • None has a try/finally, so a failure leaves them set
  • Two of the config tests asserted nothing at all

How it solves it:

  • monkeypatch.setenv and delenv unset on failure too
  • One autouse fixture replaces setup and teardown methods
  • Those two config tests now pin what registration produces

User Flow

No end-user behavior changes. A proxy admin running any of these five guardrails
gets the same request checked the same way; these are the test files around
those guardrails, not the guardrails

Relevant issues

Linear ticket

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review

Screenshots / Proof of Fix

Shared setup: clear the keys, run one test with one line forced to fail, read
the environment back in the same interpreter. A passing run is clean on both
sides, so the forced failure is the only state where they differ. probe is
this, parameterised by test id and keys:

probe() { uv run python -c '
import os, pytest, sys
keys = sys.argv[2:]
for k in keys: os.environ.pop(k, None)
pytest.main([sys.argv[1], "-q", "-p", "no:randomly"])
print("env after the run:", {k: os.environ[k] for k in keys if k in os.environ} or "clean")
' "$@"; }

$FILES below is the five test files this PR touches.

Before (ff02d5c)

1. onyx

  1. Add assert False, "forced" under assert timeout_param.read == 45.0, then
    probe tests/test_litellm/proxy/guardrails/guardrail_hooks/test_onyx.py::test_onyx_guard_with_custom_timeout_from_kwargs ONYX_API_BASE ONYX_API_KEY ONYX_TIMEOUT
1 failed, 2 warnings in 1.87s
env after the run: {'ONYX_API_BASE': 'https://test.onyx.security', 'ONYX_API_KEY': 'test-api-key'}

2. hiddenlayer

  1. Add raise AssertionError("forced") above the init_guardrails_v2( call in
    test_hiddenlayer_config_saas, then
    probe tests/test_litellm/proxy/guardrails/guardrail_hooks/test_hiddenlayer.py::test_hiddenlayer_config_saas HIDDENLAYER_API_BASE HIDDENLAYER_API_KEY
env after the run: {'HIDDENLAYER_API_BASE': 'https://my.hiddenlayer'}

3. prompt security

  1. Add assert False, "forced" under the first assert of
    test_apply_guardrail_block_request, then
    probe tests/test_litellm/proxy/guardrails/test_prompt_security_guardrails.py::test_apply_guardrail_block_request PROMPT_SECURITY_API_KEY PROMPT_SECURITY_API_BASE
1 failed, 2 warnings in 1.87s
env after the run: {'PROMPT_SECURITY_API_KEY': 'test-key', 'PROMPT_SECURITY_API_BASE': 'https://test.prompt.security'}

4. what the quality rules see

  1. for f in $FILES; do uv run python scripts/check_test_quality.py $f; done | grep -oE "TQ00[0-9]" | sort | uniq -c
   7 TQ001
   1 TQ002
   5 TQ003
  89 TQ004
  13 TQ005

After (15138fd)

1. onyx

  1. Same injection, same probe
1 failed, 2 warnings in 1.83s
env after the run: clean

2. hiddenlayer

  1. Same injection, same probe
env after the run: clean

3. prompt security

  1. Same injection, same probe
1 failed, 2 warnings in 1.86s
env after the run: clean

4. what the quality rules see

  1. for f in $FILES; do uv run python scripts/check_test_quality.py $f; done | grep -oE "TQ00[0-9]" | sort | uniq -c
   5 TQ001
   1 TQ002
  1. uv run pytest tests/test_litellm/proxy/guardrails/ -q -p no:randomly -n 4
2873 passed, 1 xfailed, 20 warnings in 14.15s

Type

🐛 Bug Fix

✅ Test

Caveats (if any)

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR improves guardrail-test isolation by replacing direct environment and global-state mutations with pytest monkeypatch cleanup.

  • Prevents environment variables from leaking when guardrail tests fail.
  • Adds registration assertions to the Onyx and Prompt Security configuration tests.
  • Tightens the test-quality budget to reflect the removed violations.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
test-quality-budget.json Lowers test-quality limits to account for violations removed by the test cleanup.
tests/test_litellm/proxy/guardrails/guardrail_hooks/test_deepkeep.py Uses monkeypatch for environment and global-state isolation.
tests/test_litellm/proxy/guardrails/guardrail_hooks/test_hiddenlayer.py Replaces direct environment mutations with failure-safe monkeypatch operations.
tests/test_litellm/proxy/guardrails/guardrail_hooks/test_onyx.py Adds failure-safe environment isolation and verifies the configured guardrail registration; the previously reported line-length issue is resolved.
tests/test_litellm/proxy/guardrails/guardrail_hooks/test_repelloai.py Isolates environment and global-state changes through monkeypatch.
tests/test_litellm/proxy/guardrails/test_prompt_security_guardrails.py Makes environment cleanup failure-safe and adds concrete registration assertions.

Reviews (3): Last reviewed commit: "test(guardrails): stop five guardrail te..." | Re-trigger Greptile

Comment thread tests/test_litellm/proxy/guardrails/guardrail_hooks/test_onyx.py
@yuneng-berri yuneng-berri changed the title test(onyx): stop the guardrail tests leaking env vars on failure test(guardrails): stop the onyx and prompt security tests leaking env vars on failure Aug 21, 2026
@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@yuneng-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

…failure

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.
@yuneng-berri
yuneng-berri force-pushed the litellm_onyx_env_leak branch from 79bafce to 15138fd Compare August 21, 2026 12:35
@yuneng-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

…itellm_onyx_env_leak

# Conflicts:
#	test-quality-budget.json
#	tests/test_litellm/proxy/guardrails/guardrail_hooks/test_deepkeep.py
#	tests/test_litellm/proxy/guardrails/guardrail_hooks/test_hiddenlayer.py
#	tests/test_litellm/proxy/guardrails/guardrail_hooks/test_onyx.py
#	tests/test_litellm/proxy/guardrails/guardrail_hooks/test_repelloai.py
#	tests/test_litellm/proxy/guardrails/test_prompt_security_guardrails.py
…itellm_onyx_env_leak

# Conflicts:
#	test-quality-budget.json
@yuneng-berri
yuneng-berri merged commit 39a580a into litellm_internal_staging Aug 22, 2026
66 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_onyx_env_leak branch August 22, 2026 04:29
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.

3 participants