From 45b133611504dc96b719e7a5e06096c06e00e186 Mon Sep 17 00:00:00 2001 From: Ishaan Jaffer Date: Wed, 6 May 2026 15:14:22 -0700 Subject: [PATCH] test(cloud-agents): write-only secret invariants (LIT-2891 validation #2) --- .../test_secrets_write_only.py | 211 ++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 tests/test_litellm/proxy/agent_settings_endpoints/test_secrets_write_only.py diff --git a/tests/test_litellm/proxy/agent_settings_endpoints/test_secrets_write_only.py b/tests/test_litellm/proxy/agent_settings_endpoints/test_secrets_write_only.py new file mode 100644 index 00000000000..1e895a765d8 --- /dev/null +++ b/tests/test_litellm/proxy/agent_settings_endpoints/test_secrets_write_only.py @@ -0,0 +1,211 @@ +""" +Write-only secret invariants (LIT-2891 validation #2). + +The single most important property of `/v2/agent-secrets`: a stored secret +value MUST never reappear on any GET response, ever. The defense is layered: + +* **Type-level**: `AgentSecretResponse` has no `value` field. Even an + accidental `**row` splat can't surface the ciphertext through the model. +* **Endpoint-level**: every `response_model` annotation references one of the + value-free models, and `_row_to_response` reads named columns explicitly + rather than splatting the whole Prisma row. +* **Schema-level**: `AgentSecretListResponse` only nests + `AgentSecretResponse`, so a list response can't smuggle values either. + +These tests pin all three so a future refactor can't quietly weaken the +contract. They deliberately avoid importing the endpoints module (which +would pull in the full proxy auth stack and its `orjson` dependency) — we +stress the type contract directly and grep the endpoint source as text. +""" + +import pathlib + +import pytest +from pydantic import ValidationError + +from litellm.proxy.agent_settings_endpoints.types import ( + AgentSecretCreateRequest, + AgentSecretListResponse, + AgentSecretResponse, + AgentSecretUpdateRequest, +) + +_SECRETS_ENDPOINTS_PATH = ( + pathlib.Path(__file__).resolve().parents[4] + / "litellm" + / "proxy" + / "agent_settings_endpoints" + / "secrets_endpoints.py" +) + + +def _read_endpoints_source() -> str: + # Read as text instead of importing the module — keeps these tests + # collectable without the full proxy dependency tree (orjson, prisma, + # etc.) installed. + assert ( + _SECRETS_ENDPOINTS_PATH.exists() + ), f"secrets_endpoints.py missing at {_SECRETS_ENDPOINTS_PATH}" + return _SECRETS_ENDPOINTS_PATH.read_text() + + +def _strip_docstrings_and_comments(source: str) -> str: + """Remove triple-quoted strings and `# ...` line comments. + + Used by the source-grep tests to avoid tripping on legitimate mentions + of `value_enc` inside docstrings (e.g. "intentionally NOT read here") + and inline explanatory comments. Crude regex is fine for our use — + the source file is small and we're checking a denylist. + """ + import re + + # Triple-quoted strings (greedy across lines). + no_docstrings = re.sub(r'"""[\s\S]*?"""', "", source) + no_docstrings = re.sub(r"'''[\s\S]*?'''", "", no_docstrings) + # `# ...` line comments. + no_comments = re.sub(r"#[^\n]*", "", no_docstrings) + return no_comments + + +class TestResponseModelHasNoValueField: + """The type system itself enforces validation #2.""" + + def test_agent_secret_response_has_no_value_field(self): + assert "value" not in AgentSecretResponse.model_fields + assert "value_enc" not in AgentSecretResponse.model_fields + + def test_response_model_dump_never_contains_value(self): + # Even if a future refactor added `value` to the row dict, the + # response schema would silently drop it (Pydantic ignores extras + # by default for BaseModel). + resp = AgentSecretResponse( + name="X", + scope="all", + type="env", + file_path=None, + created_at="now", + updated_at="now", + ) + dumped = resp.model_dump() + assert "value" not in dumped + assert "value_enc" not in dumped + + def test_list_response_only_nests_value_free_models(self): + # A list response is a wrapper around AgentSecretResponse — assert + # the nested type has the right shape. + resp = AgentSecretListResponse(secrets=[]) + dumped = resp.model_dump() + assert dumped == {"secrets": []} + # The model_fields annotation must be List[AgentSecretResponse]. + secrets_field = AgentSecretListResponse.model_fields["secrets"] + annotation = str(secrets_field.annotation) + assert "AgentSecretResponse" in annotation + + def test_response_model_drops_value_extra_via_validation(self): + # Round-trip through validate to confirm Pydantic strips an extra + # `value` even if a caller tries to slip it past the type. + raw = { + "name": "X", + "scope": "all", + "type": "env", + "file_path": None, + "created_at": "t", + "updated_at": "t", + "value": "DO-NOT-LEAK", + } + resp = AgentSecretResponse.model_validate(raw) + assert "value" not in resp.model_dump() + assert "DO-NOT-LEAK" not in str(resp.model_dump()) + + +class TestRequestModelsAcceptValue: + """The flip side: write-only means values DO go in on POST/PUT.""" + + def test_create_request_requires_value(self): + with pytest.raises(ValidationError): + AgentSecretCreateRequest(name="X", value="") # too short + with pytest.raises(ValidationError): + AgentSecretCreateRequest(name="X") # missing entirely + + def test_create_request_validates_name_charset(self): + # We use names as env-var keys, so they must follow shell-safe + # identifier rules. Reject names with dashes or starting with a digit. + with pytest.raises(ValidationError): + AgentSecretCreateRequest(name="bad-name", value="x") + with pytest.raises(ValidationError): + AgentSecretCreateRequest(name="9starting_digit", value="x") + # Valid identifier passes. + AgentSecretCreateRequest(name="OPENAI_API_KEY", value="x") + + def test_update_request_value_is_optional(self): + # PUT is partial — UI may send only a scope change. + body = AgentSecretUpdateRequest(scope="all") + assert body.value is None + + def test_update_request_value_min_length_1(self): + with pytest.raises(ValidationError): + AgentSecretUpdateRequest(value="") + + +class TestEndpointSourceContainsNoValueLeak: + """Belt-and-suspenders: scan the secrets endpoint source for accidental + references that could leak `value_enc` onto a response. If a future PR + introduces something like `response['value'] = decrypt(...)`, these + tests catch it before it ships. + """ + + def test_no_decrypt_call_in_endpoints_module(self): + source = _read_endpoints_source() + # The secrets module must not import the decrypt helper — only the + # VM config endpoints module needs it (for Test Connection). + assert "decrypt_optional" not in source + assert "decrypt_value_helper" not in source + + def test_endpoints_use_value_free_response_models(self): + source = _read_endpoints_source() + # Both response_model occurrences should reference the value-free + # types — never some bare dict that could drift. + assert "response_model=AgentSecretResponse" in source + assert "response_model=AgentSecretListResponse" in source + + def test_row_to_response_does_not_read_value_enc(self): + source = _read_endpoints_source() + # Find the helper that maps DB rows to API responses. The chokepoint + # is the GET path: it MUST NOT read value_enc. Write-side payload + # builders (create/update) are allowed to mention it because they + # write the ciphertext into the DB. + helper_marker = "def _row_to_response(row:" + helper_start = source.find(helper_marker) + assert helper_start != -1, ( + "_row_to_response helper missing — refactor must not lose this " + "chokepoint." + ) + next_def = source.find("\ndef ", helper_start + len(helper_marker)) + helper_body = ( + source[helper_start:] if next_def == -1 else source[helper_start:next_def] + ) + # Strip docstrings and `# ...` comments before checking. The helper's + # docstring legitimately calls out "value_enc is intentionally NOT + # read here", and we don't want that mention to trip the test. + stripped = _strip_docstrings_and_comments(helper_body) + assert "value_enc" not in stripped, ( + "_row_to_response references value_enc — the GET path must " + "never touch the ciphertext column." + ) + + def test_no_response_dict_assigns_value(self): + # Make sure no code path builds a response dict and adds `value` to + # it. (`response_model` enforces this on FastAPI's side, but we + # double-check the module doesn't construct such dicts directly.) + source = _read_endpoints_source() + # Disallow patterns like `"value": ...,` in response-shaped dicts. + # Allow `body.value` (a request field). The simplest portable rule + # is: the literal `"value":` must never appear in the source. + assert ( + '"value":' not in source + ), "secrets endpoint constructs a dict with a `value` key — possible leak." + + +@pytest.fixture(autouse=True) +def _no_op_fixture(): + yield