diff --git a/.github/e2e-stack/select_tests.py b/.github/e2e-stack/select_tests.py index 633ee6d8a23..52013994381 100644 --- a/.github/e2e-stack/select_tests.py +++ b/.github/e2e-stack/select_tests.py @@ -7,7 +7,8 @@ UNSUPPORTED: Final = re.compile( r"^tests/e2e/(ui|claude_code|load)/" r"|^tests/e2e/llm_translation/realtime/test_realtime_pipecat_audio_e2e\.py$" r"|^tests/e2e/batches/test_managed_files_enforcement_e2e\.py$" - r"|^tests/e2e/guardrails/test_presidio_.*_e2e\.py$" + r"|^tests/e2e/guardrails/test_presidio_masking_e2e\.py$" + r"|^tests/e2e/guardrails/test_presidio_contract_e2e\.py$" ) HARNESS: Final = re.compile( r"^tests/e2e/[A-Za-z0-9_.-]+\.(py|ini)$" diff --git a/tests/e2e/CLAUDE.md b/tests/e2e/CLAUDE.md index 89c04208d65..a0c4835f732 100644 --- a/tests/e2e/CLAUDE.md +++ b/tests/e2e/CLAUDE.md @@ -203,11 +203,23 @@ logging... guardrail... provider : presidio | lakera | bedrock | aporia | ... - hook_point : pre_call | post_call | during | logging_only - assertion : blocks | masks | allows + hook_point : pre_call | post_call | during | logging_only | apply_endpoint + assertion : blocks | masks | allows + masks_only_configured_entities | masks_each_entity_in_place + | restores_masked_values + logs_masked_entities + fails_closed_when_unreachable e.g. guardrail.presidio.pre_call.masks exercised_on=[chat_completions] + guardrail.presidio.pre_call.fails_closed_when_unreachable exercised_on=[chat_completions] ``` +The verdict names say what the guardrail decided, and are the only ones a new +provider normally needs. The rest exist because a guardrail that talks to an +outside service has a contract with it beyond the verdict: which entity types it +asked for, where the spans it got back were applied, what it recorded for an +operator, and what it does when that service is unreachable. Reach for one of +those only when the case pins that contract rather than the decision + Other - holding pen (endpoint or behavior) ``` diff --git a/tests/e2e/guardrails/test_presidio_contract_e2e.py b/tests/e2e/guardrails/test_presidio_contract_e2e.py index 4ce1a7377ce..e9c973a4fdf 100644 --- a/tests/e2e/guardrails/test_presidio_contract_e2e.py +++ b/tests/e2e/guardrails/test_presidio_contract_e2e.py @@ -24,6 +24,7 @@ as secrets; see presidio_env. from __future__ import annotations +import re import time from collections.abc import Callable from typing import Final, Literal @@ -53,10 +54,16 @@ RAW_PHONE = "+1 415-555-0134" RAW_CARD = "4111 1111 1111 1111" PII_SENTENCE = f"Reach Dana at {RAW_EMAIL} or on {RAW_PHONE} about card {RAW_CARD} today." -UNREACHABLE_BASE = "http://127.0.0.1:9/" +UNREACHABLE_BASE = "http://presidio-unreachable.invalid/" +_UNREACHABLE_HOST: Final = "presidio-unreachable.invalid" MAX_ECHO_TOKENS = 128 +_NUMBERED_TOKEN: Final = re.compile(r"<[A-Z_]+_\d+>") +_ECHOED_THE_LINE: Final = re.compile( + rf"{re.escape(RAW_EMAIL)}|{re.escape(RAW_PHONE)}|<(?:EMAIL_ADDRESS|PHONE_NUMBER)_\d+>" +) + def _register( client: GuardrailsClient, @@ -267,11 +274,11 @@ class TestPresidioOutputParseContract: ) restored: Final = _poll_until_restored(client, scoped_key, name, _echo_prompt(unique_marker())) - assert RAW_EMAIL in restored, ( - "the caller must get the real address back, not the placeholder the model saw; a token left " - f"in the answer means the mapping was lost between the two hooks: {scrub(restored)!r}" + assert RAW_EMAIL in restored and RAW_PHONE in restored, ( + "the caller must get every masked value back, not the placeholders the model saw; a token " + f"left in the answer means the mapping was lost between the two hooks: {scrub(restored)!r}" ) - assert "EMAIL_ADDRESS_1" not in restored, ( + assert not _NUMBERED_TOKEN.search(restored), ( f"no numbered token may survive into the caller's response: {scrub(restored)!r}" ) @@ -297,6 +304,11 @@ def _poll_until_restored(client: GuardrailsClient, key: str, name: str, prompt: The applied-guardrails header is the liveness gate: without it a response carrying the raw address would be indistinguishable from one the guardrail never touched, and the restore assertion would pass vacuously. + + Polling stops as soon as the model has echoed the line in either form, + restored or still carrying a token, because the restore runs per request and + a token that survived it is a verdict rather than something a retry fixes. + Only a model that never echoed at all is worth waiting out. """ deadline: Final = time.monotonic() + POLL_TIMEOUT last = "" # rebind-ok: last-observation accumulator for the failure message @@ -304,7 +316,7 @@ def _poll_until_restored(client: GuardrailsClient, key: str, name: str, prompt: outcome = client.chat_raw(key, MODEL, prompt, guardrails=[name], max_tokens=MAX_ECHO_TOKENS) if outcome.ok and name in outcome.headers.get("x-litellm-applied-guardrails", ""): last = _first_content(ChatResponse.model_validate_json(outcome.body)) - if RAW_EMAIL in last or "EMAIL_ADDRESS" in last: + if _ECHOED_THE_LINE.search(last): return last else: last = f"" @@ -326,11 +338,17 @@ class TestPresidioUnreachableContract: ) -> None: """A Presidio outage must not degrade into "no masking today". - The guardrail is pointed at a closed local port so the analyzer call - cannot connect. With PII entities configured the guardrail fails closed - and the prompt never reaches the model with the address still in it. The - error goes straight back to the caller, so it is also asserted to carry - neither the raw address nor the endpoint it failed to reach. + The guardrail is pointed at a host in the reserved .invalid domain, so + the analyzer call fails to resolve immediately. With PII entities + configured the guardrail fails closed and the prompt never reaches the + model with the address still in it. + + The error goes straight back to the caller, so it is also asserted to + carry neither the raw address nor the endpoint it failed to reach. That + second assertion is why the unreachable base is a hostname rather than a + loopback address: the proxy already rewrites IPs out of error messages, + which would make an endpoint-disclosure assertion pass no matter what, + and a real deployment is addressed by hostname anyway. """ name: Final = f"e2e-presidio-unreachable-{unique_marker()}" _register( @@ -353,7 +371,11 @@ class TestPresidioUnreachableContract: ) body: Final = scrub(str(outcome)) assert RAW_EMAIL not in body, f"the failure must not echo the prompt's PII back to the caller: {body}" + assert UNREACHABLE_BASE not in body and _UNREACHABLE_HOST not in body, ( + "the failure must not name the endpoint the guardrail could not reach; on a real deployment " + f"that address is a secret and this body is rendered straight back to the caller: {body}" + ) assert "Presidio" in body, ( - f"the refusal must say the guardrail could not run, or an operator cannot tell an outage " + "the refusal must say the guardrail could not run, or an operator cannot tell an outage " f"from a model error: {body}" )