From 3b9742a34a58b7bc90ca64b924b7964fd567b937 Mon Sep 17 00:00:00 2001 From: Yucheng He Date: Mon, 7 Sep 2026 20:28:33 -0700 Subject: [PATCH] fix(hide-secrets): read the assignments in a prompt that is mostly prose configparser aborts the whole parse on the first line it cannot read, so a message like "Here is my config, can you review it?" followed by REDIS_PASSWORD=... lost every assignment to that one prose line. Hand the parser only the lines it can read, dedent the assignments inside a pasted config, and keep each key distinct by line number so a config naming api_key once per model keeps every value instead of only the last. --- .../enterprise_callbacks/secret_detection.py | 41 +++++++++- .../test_secret_detection.py | 77 ++++++++++++++++++- 2 files changed, 115 insertions(+), 3 deletions(-) diff --git a/enterprise/litellm_enterprise/enterprise_callbacks/secret_detection.py b/enterprise/litellm_enterprise/enterprise_callbacks/secret_detection.py index dc79fdbb056..431a8212507 100644 --- a/enterprise/litellm_enterprise/enterprise_callbacks/secret_detection.py +++ b/enterprise/litellm_enterprise/enterprise_callbacks/secret_detection.py @@ -448,6 +448,8 @@ _default_detect_secrets_config = { _CONFIG_SECTION: Final = "litellm-prompt" +_ASSIGNMENT_LINE: Final = re.compile(r"[^\s\[#;:=][^:=]*[:=]") + # A .py suffix keeps detect_secrets' own config transformers off this file (they only fire on # FileType.OTHER and FileType.YAML) while leaving every plugin's regex set unchanged. _SCAN_SUFFIX: Final = ".py" @@ -480,6 +482,38 @@ def _scan_lines(lines: Sequence[str]) -> frozenset[tuple[str, str]]: ) +def _parseable_lines(text: str) -> Generator[str, None, None]: + """Yields the lines of ``text`` that configparser can read. + + A prompt is mostly prose, and one unreadable line aborts the whole parse, so the prose + is dropped rather than allowed to take the assignments down with it. + """ + open_option = False + for number, line in enumerate(text.splitlines()): + stripped = line.strip() + assignment = _ASSIGNMENT_LINE.match(stripped) + if not stripped: + yield line + elif stripped[0] in "#;": + open_option = False + yield line + elif stripped[0] == "[": + open_option = False + # configparser needs a closing bracket and something inside it; without one + # it aborts the whole parse, taking every assignment below down with it. + if "]" in stripped[2:]: + yield line + elif assignment is not None: + open_option = True + # Dedenting reaches the assignments inside a pasted config, and the line number + # keeps every key distinct so a config repeating api_key per model keeps them all. + yield f"{assignment.group()[:-1].strip()}_{number}{stripped[assignment.end() - 1:]}" + elif line[0].isspace() and open_option: + yield line + else: + open_option = False + + def _quoted_assignments(text: str) -> tuple[str, ...]: """Rewrites the bare ``key = value`` assignments in ``text`` as quoted ones. @@ -487,11 +521,12 @@ def _quoted_assignments(text: str) -> tuple[str, ...]: nothing, so one vendor-prefixed key in a message hides every unquoted assignment beside it. Interpolation stays off so that no emitted value is one the message never contained. """ - parser: Final = configparser.ConfigParser(interpolation=None) + parser: Final = configparser.ConfigParser(interpolation=None, strict=False) # Keys keep their case, exactly as detect_secrets' own parser does. parser.optionxform = str # pyright: ignore[reportAttributeAccessIssue] # configparser types optionxform as a method + body: Final = "\n".join(_parseable_lines(text)) try: - parser.read_string(f"[{_CONFIG_SECTION}]\n{text}") + parser.read_string(f"[{_CONFIG_SECTION}]\n{body}") except (configparser.Error, UnicodeDecodeError): return () @@ -500,6 +535,8 @@ def _quoted_assignments(text: str) -> tuple[str, ...]: for section in parser for key, values in parser.items(section) for value in values.splitlines() + # A quoted value is already visible to every plugin in the raw text, and + # re-quoting it here only invents matches the message never held. if value and '"' not in value ) diff --git a/tests/test_litellm/enterprise/enterprise_callbacks/test_secret_detection.py b/tests/test_litellm/enterprise/enterprise_callbacks/test_secret_detection.py index 508374c9f5d..e3fe98913ab 100644 --- a/tests/test_litellm/enterprise/enterprise_callbacks/test_secret_detection.py +++ b/tests/test_litellm/enterprise/enterprise_callbacks/test_secret_detection.py @@ -150,6 +150,9 @@ def test_scan_message_redacts_credentials_assigned_to_credential_keys(content, s "private_key_path: keys/prod/server-cert.pem", "litellm.completion(model=model, api_key=openai_api_key)", "params['aws_secret_access_key'] = aws_secret_access_key", + 'api_key = "OPENAI_API_KEY"', + "model_list:\n - litellm_params:\n api_key: 'PERPLEXITY_API_KEY'", + 'config = build(_provider("ve_missing", api_key_env="VE_MISSING_KEY"))', ], ids=[ "prose-password", @@ -183,6 +186,9 @@ def test_scan_message_redacts_credentials_assigned_to_credential_keys(content, s "path-under-credential-key", "snake-case-argument", "snake-case-assignment", + "quoted-env-variable-name", + "quoted-env-name-in-a-config", + "quoted-env-name-in-a-code-paste", ], ) def test_scan_message_keeps_benign_values(content): @@ -247,6 +253,49 @@ def test_credential_keyword_detector_rejects_an_unusable_minimum_length(minimum_ guardrail.scan_message_for_secrets("password=aB3dE6gH9jK2mN5p") +@pytest.mark.parametrize( + "content", + [ + "[db\npassword = Zx4Kp9Lm2Qr7Ns3Vt\n", + "[\npassword = Zx4Kp9Lm2Qr7Ns3Vt\n", + "[note] have a look\npassword = Zx4Kp9Lm2Qr7Ns3Vt\n", + "]\npassword = Zx4Kp9Lm2Qr7Ns3Vt\n", + "[]\npassword = Zx4Kp9Lm2Qr7Ns3Vt\n", + ], + ids=["unclosed", "bare-bracket", "bracketed-prose", "stray-close", "empty-header"], +) +def test_scan_message_reads_a_config_with_a_broken_section_header(content): + """A half-typed section header must not take the assignments below it down with it.""" + guardrail = _guardrail() + + assert "Zx4Kp9Lm2Qr7Ns3Vt" not in guardrail.redact_text(content) + + +def test_scan_message_reads_a_config_that_repeats_a_section(): + """A pasted ini can name the same section twice, and refusing to parse it would drop + every assignment in the message, not just the repeated one.""" + guardrail = _guardrail() + content = "[db]\nhost = localhost\n[db]\npassword = Zx4Kp9Lm2Qr7Ns3Vt\n" + + assert "Zx4Kp9Lm2Qr7Ns3Vt" not in guardrail.redact_text(content) + + +def test_scan_message_keeps_every_value_when_a_config_repeats_a_key(): + """A litellm config names api_key once per model, so keeping only the last one would + leave every earlier model's credential in the prompt.""" + guardrail = _guardrail() + content = ( + "model_list:\n" + " - model_name: gpt-4o\n litellm_params:\n api_key: aB3dE6gH9jK2mN5p\n" + " - model_name: claude\n litellm_params:\n api_key: Kp7Nq2Wz9Bt4Xr6Vm1Ls\n" + ) + + redacted = guardrail.redact_text(content) + + assert "aB3dE6gH9jK2mN5p" not in redacted + assert "Kp7Nq2Wz9Bt4Xr6Vm1Ls" not in redacted + + @pytest.mark.parametrize( "content,secret", [ @@ -262,8 +311,34 @@ def test_credential_keyword_detector_rejects_an_unusable_minimum_length(minimum_ f"api_key: {OPENAI_KEY}\npassword =\n Zx4Kp9Lm2Qr7Ns3Vt", "Zx4Kp9Lm2Qr7Ns3Vt", ), + ( + "Here is my config, can you review it?\nREDIS_PASSWORD=aB3dE6gH9jK2mN5p", + "aB3dE6gH9jK2mN5p", + ), + ( + "REDIS_PASSWORD=aB3dE6gH9jK2mN5p\nCan you tell me what is wrong with it?", + "aB3dE6gH9jK2mN5p", + ), + ( + "Hi team\nplease rotate this before Friday\n" + "db_password=Zx4Kp9Lm2Qr7Ns3Vt\nthanks!", + "Zx4Kp9Lm2Qr7Ns3Vt", + ), + ( + "model_list:\n - model_name: gpt-4o\n litellm_params:\n" + " api_key: aB3dE6gH9jK2mN5p\n", + "aB3dE6gH9jK2mN5p", + ), + ], + ids=[ + "flat-assignment", + "env-file", + "continuation-line", + "prose-before", + "prose-after", + "prose-both-sides", + "indented-config", ], - ids=["flat-assignment", "env-file", "continuation-line"], ) def test_scan_message_still_sees_assignments_sharing_a_message_with_a_vendor_key( content, secret