Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion openrag/api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,10 @@ def get_config():
instead of assuming every admin has partition access.
"""
from api.dependencies.auth import SUPER_ADMIN_MODE
from core.utils.redaction import redact_secrets
from fastapi.encoders import jsonable_encoder

return {**jsonable_encoder(settings), "super_admin_mode": SUPER_ADMIN_MODE}
return {**redact_secrets(jsonable_encoder(settings)), "super_admin_mode": SUPER_ADMIN_MODE}


# Router mounts. Phase 10F finished moving these into
Expand Down
9 changes: 8 additions & 1 deletion openrag/api/routers/admin/model_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,17 @@ async def validate_endpoint_draft(
):
"""Probe arbitrary endpoint values (before they are saved) for reachability
and model availability."""
api_key = body.api_key
if api_key is None and body.stored_api_key_model_type and body.stored_api_key_name:
endpoint = await service.get_model_endpoint(
name=body.stored_api_key_name,
model_type=body.stored_api_key_model_type,
)
api_key = endpoint.extra.get("api_key")
return await service.validate_endpoint(
url=body.endpoint,
model_name=body.model_name,
api_key=body.api_key,
api_key=api_key,
)


Expand Down
33 changes: 33 additions & 0 deletions openrag/api/schemas/admin/model_endpoint_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from datetime import datetime
from typing import Any, Literal

from core.utils.redaction import redact_secret_mapping
from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator

ModelEndpointType = Literal["embedder", "reranker", "llm", "vlm"]
Expand Down Expand Up @@ -98,17 +99,49 @@ class ModelEndpointResponse(BaseModel):
batch_size: int
timeout: float
extra: dict[str, Any]
has_api_key: bool = False
is_default: bool
created_at: datetime
updated_at: datetime

@model_validator(mode="before")
@classmethod
def redact_secret_extra(cls, value: Any) -> Any:
if hasattr(value, "model_dump"):
data = value.model_dump()
elif isinstance(value, dict):
data = dict(value)
else:
data = dict(value)
extra = dict(data.get("extra") or {})
data["has_api_key"] = bool(extra.get("api_key"))
data["extra"] = redact_secret_mapping(extra)
return data


class ValidateEndpointRequest(BaseModel):
"""Request body to validate endpoint values before they are saved (draft)."""

endpoint: str
model_name: str | None = None
api_key: str | None = None
stored_api_key_model_type: ModelEndpointType | None = None
stored_api_key_name: str | None = None

@field_validator("stored_api_key_name")
@classmethod
def validate_stored_api_key_name(cls, value: str | None) -> str | None:
"""Normalize the optional saved endpoint name used as credential source."""
return _normalize_name(value) if value is not None else None

@model_validator(mode="after")
def require_complete_stored_api_key_source(self) -> ValidateEndpointRequest:
"""Require both fields when draft validation reuses a stored key."""
has_type = self.stored_api_key_model_type is not None
has_name = self.stored_api_key_name is not None
if has_type != has_name:
raise ValueError("stored_api_key_model_type and stored_api_key_name must be provided together")
return self


class ValidateEndpointResponse(BaseModel):
Expand Down
92 changes: 92 additions & 0 deletions openrag/core/utils/redaction.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
"""Helpers for shaping public data without exposing secrets."""

from __future__ import annotations

from collections.abc import Mapping
from typing import Any

REDACTED_SECRET = "<redacted>"

SECRET_FIELD_NAMES = frozenset(
{
"api_key",
"api_token",
"auth_token",
"chainlit_auth_secret",
"client_secret",
"hf_token",
"oidc_client_secret",
"oidc_token_encryption_key",
"password",
"secret",
"secret_key",
"token",
"token_encryption_key",
}
)


def is_secret_field(key: str) -> bool:
"""Return true only for known secret field names, not fuzzy token matches."""
return key.lower() in SECRET_FIELD_NAMES


def redact_secrets(value: Any) -> Any:
"""Recursively redact values for known secret fields without mutating input."""
if isinstance(value, Mapping):
return {
key: REDACTED_SECRET if is_secret_field(str(key)) else redact_secrets(item) for key, item in value.items()
}
if isinstance(value, list):
return [redact_secrets(item) for item in value]
if isinstance(value, tuple):
return tuple(redact_secrets(item) for item in value)
return value


def redact_secret_mapping(extra: Mapping[str, Any] | None) -> dict[str, Any]:
"""Drop secret fields from endpoint extras while preserving non-secret metadata."""
return {key: redact_secrets(item) for key, item in dict(extra or {}).items() if not is_secret_field(str(key))}


def preserve_existing_secrets(existing: Mapping[str, Any] | None, incoming: Mapping[str, Any]) -> dict[str, Any]:
"""Keep stored secrets when an update payload omits or echoes a redacted value."""
return _preserve_existing_secrets(existing or {}, incoming)


def _preserve_existing_secrets(existing: Mapping[str, Any], incoming: Mapping[str, Any]) -> dict[str, Any]:
merged = dict(incoming)
for key, value in existing.items():
incoming_value = merged.get(key)
if is_secret_field(str(key)):
if key not in merged or incoming_value == REDACTED_SECRET:
merged[key] = value
continue
if isinstance(value, Mapping) and isinstance(incoming_value, Mapping):
merged[key] = _preserve_existing_secrets(value, incoming_value)
elif isinstance(value, list) and isinstance(incoming_value, list):
merged[key] = _preserve_existing_secret_lists(value, incoming_value)
return merged


def _preserve_existing_secret_lists(existing: list[Any], incoming: list[Any]) -> list[Any]:
merged = list(incoming)
for index, existing_item in enumerate(existing):
if index >= len(merged):
break
incoming_item = merged[index]
if isinstance(existing_item, Mapping) and isinstance(incoming_item, Mapping):
merged[index] = _preserve_existing_secrets(existing_item, incoming_item)
Comment thread
hedhoud marked this conversation as resolved.
Resolved
elif isinstance(existing_item, list) and isinstance(incoming_item, list):
merged[index] = _preserve_existing_secret_lists(existing_item, incoming_item)
return merged


__all__ = [
"REDACTED_SECRET",
"SECRET_FIELD_NAMES",
"is_secret_field",
"preserve_existing_secrets",
"redact_secret_mapping",
"redact_secrets",
]
4 changes: 4 additions & 0 deletions openrag/services/orchestrators/model_endpoint_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from core.config.model_endpoints import ModelEndpointConfig, ModelEndpointRow
from core.utils.exceptions import NotFoundError, ValidationError
from core.utils.logging import get_logger
from core.utils.redaction import preserve_existing_secrets

if TYPE_CHECKING:
from core.config.root import Settings
Expand Down Expand Up @@ -245,6 +246,9 @@ async def update_model_endpoint(self, name: str, model_type: str, **fields: obje
# default by promoting another endpoint, never by leaving the type with none.
promote_to_default = bool(fields.pop("is_default", None))

if isinstance(fields.get("extra"), dict):
fields["extra"] = preserve_existing_secrets(existing.extra, fields["extra"]) # type: ignore[arg-type]

if fields:
updated = await self._repo.update(name, model_type, **fields)
else:
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/api/test_model_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ def test_model_endpoint_crud_validate_and_default_selection(api_client):
},
)
assert create_endpoint.status_code == 201, create_endpoint.text
assert create_endpoint.json()["extra"]["api_key"] == "ci-test-key"
assert "ci-test-key" not in create_endpoint.text
assert create_endpoint.json()["has_api_key"] is True
assert "api_key" not in create_endpoint.json()["extra"]

validate_missing = api_client.post(f"/model-endpoints/llm/{endpoint_name}/validate")
_assert_success(validate_missing, context="validate missing model")
Expand Down
44 changes: 44 additions & 0 deletions tests/unit/api/routers/admin/test_phase14_admin_routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,26 @@ async def test_update_model_endpoint_maps_name_to_new_name(async_client_factory)
]


@pytest.mark.asyncio
async def test_model_endpoint_read_responses_hide_stored_api_key(async_client_factory):
model_service = FakeModelEndpointService()
model_service.endpoint_extra = {
"implementation": "vllm",
"api_key": "secret-token",
"temperature": 0.2,
}
app = _build_app(model_service=model_service)

async with async_client_factory(app) as client:
response = await client.get("/model-endpoints/llm/default")

assert response.status_code == 200
payload = response.json()
assert "secret-token" not in response.text
assert payload["has_api_key"] is True
assert payload["extra"] == {"implementation": "vllm", "temperature": 0.2}


@pytest.mark.asyncio
async def test_validate_model_endpoint_uses_route_identity(async_client_factory):
"""Endpoint validation should resolve route identity before probing."""
Expand Down Expand Up @@ -285,6 +305,30 @@ async def test_validate_endpoint_draft_forwards_body_without_lookup(async_client
]


@pytest.mark.asyncio
async def test_validate_endpoint_draft_can_reuse_stored_api_key(async_client_factory):
model_service = FakeModelEndpointService()
model_service.endpoint_extra = {"api_key": "secret-token"}
app = _build_app(model_service=model_service)

async with async_client_factory(app) as client:
response = await client.post(
"/model-endpoints/validate",
json={
"endpoint": "http://candidate:8000/v1",
"model_name": "mistral-small",
"stored_api_key_model_type": "llm",
"stored_api_key_name": "default",
},
)

assert response.status_code == 200
assert model_service.calls == [
("get", {"name": "default", "model_type": "llm"}),
("validate", {"url": "http://candidate:8000/v1", "model_name": "mistral-small", "api_key": "secret-token"}),
]


@pytest.mark.asyncio
async def test_validate_endpoint_draft_defaults_optional_fields(async_client_factory):
"""``model_name`` and ``api_key`` are optional in the draft body."""
Expand Down
25 changes: 25 additions & 0 deletions tests/unit/api/test_secret_redaction.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
from __future__ import annotations


def test_redact_secrets_hides_known_secret_keys_without_false_token_matches():
from core.utils.redaction import REDACTED_SECRET, redact_secrets

payload = {
"llm": {"api_key": "llm-secret", "model": "mistral"},
"rdb": {"password": "db-secret", "host": "rdb"},
"websearch": {"api_token": "search-secret", "max_tokens": 2048},
"oidc_client_secret": "oidc-secret",
"chainlit_auth_secret": "chainlit-secret",
"nested": [{"token_encryption_key": "fernet-secret"}],
}

redacted = redact_secrets(payload)

assert redacted["llm"]["api_key"] == REDACTED_SECRET
assert redacted["rdb"]["password"] == REDACTED_SECRET
assert redacted["websearch"]["api_token"] == REDACTED_SECRET
assert redacted["websearch"]["max_tokens"] == 2048
assert redacted["oidc_client_secret"] == REDACTED_SECRET
assert redacted["chainlit_auth_secret"] == REDACTED_SECRET
assert redacted["nested"][0]["token_encryption_key"] == REDACTED_SECRET
assert payload["llm"]["api_key"] == "llm-secret"
66 changes: 66 additions & 0 deletions tests/unit/services/orchestrators/test_model_endpoint_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,72 @@ async def test_update_is_default_false_does_not_demote_or_touch_default_cache():
assert cache.get("default") is sentinel


@pytest.mark.asyncio
async def test_update_extra_without_api_key_preserves_existing_secret():
existing = _make_row(
name="jina",
extra={"implementation": "vllm", "api_key": "stored-key", "temperature": 0.1},
)
repo = _FakeEndpointRepo(rows=[existing])
svc = _make_service(repo)

await svc.update_model_endpoint("jina", "embedder", extra={"implementation": "vllm", "temperature": 0.2})

updated = repo._store[("jina", "embedder")]
assert updated.extra == {"implementation": "vllm", "temperature": 0.2, "api_key": "stored-key"}


@pytest.mark.asyncio
async def test_update_extra_preserves_nested_redacted_secrets():
existing = _make_row(
name="jina",
extra={
"implementation": "vllm",
"auth": {
"token": "stored-token",
"headers": [{"api_key": "nested-key"}],
},
},
)
repo = _FakeEndpointRepo(rows=[existing])
svc = _make_service(repo)

await svc.update_model_endpoint(
"jina",
"embedder",
extra={
"implementation": "vllm",
"auth": {
"token": "<redacted>",
"headers": [{"api_key": "<redacted>"}],
},
"temperature": 0.2,
},
)

updated = repo._store[("jina", "embedder")]
assert updated.extra == {
"implementation": "vllm",
"auth": {
"token": "stored-token",
"headers": [{"api_key": "nested-key"}],
},
"temperature": 0.2,
}


@pytest.mark.asyncio
async def test_update_extra_with_new_api_key_rotates_existing_secret():
existing = _make_row(name="jina", extra={"implementation": "vllm", "api_key": "old-key"})
repo = _FakeEndpointRepo(rows=[existing])
svc = _make_service(repo)

await svc.update_model_endpoint("jina", "embedder", extra={"implementation": "vllm", "api_key": "new-key"})

updated = repo._store[("jina", "embedder")]
assert updated.extra == {"implementation": "vllm", "api_key": "new-key"}


# ------------------------------------------------------------------
# delete_model_endpoint
# ------------------------------------------------------------------
Expand Down
Loading
Loading