fix(models): reject '/' in model-endpoint names

openrag/api/schemas/admin/model_endpoint_schemas.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import re
 from datetime import datetime
 from typing import Any, Literal
 
@@ -15,12 +16,43 @@
 # ``extra`` — validated here so a typo can't persist a nonsensical value.
 _LLM_TOKEN_EXTRA_KEYS = (LLM_CONTEXT_SIZE_KEY, LLM_OUTPUT_TOKENS_KEY)
 
+# Allowlist, not a denylist: `name` is a single path segment in every
+# single-endpoint route (see `_normalize_name`), and enumerating unsafe values
+# one at a time as they're discovered — first `/` (#768), then the RFC 3986
+# dot-segments `.`/`..` — never closes the class. Anchoring both ends on
+# alphanumeric rules out `/`, `.`, `..`, and any leading/trailing separator by
+# construction, while `.`/`_`/`-` stay available in the middle for realistic
+# names like `gpt-4.1` or `jina_v3`.
+_NAME_PATTERN = re.compile(r"[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?")
+_NAME_MAX_LENGTH = 128
+
 
 def _normalize_name(value: str) -> str:
-    """Trim a user-facing registry name and reject blank values."""
+    """Trim a user-facing registry name and reject any value unsafe as a URL path segment.
+
+    ``name`` is embedded as a single path segment in every single-endpoint route
+    (``GET/PUT/DELETE /model-endpoints/{model_type}/{name}``, ``.../set-default``,
+    ``.../reveal-api-key``, ``.../validate``). A value outside ``_NAME_PATTERN``
+    — a ``/`` (splits across path segments), the exact values ``.``/``..``
+    (RFC 3986 dot-segments: browsers and HTTP clients normalize these out of
+    the URL before the request is even sent, resolving to the collection route
+    or dropping the ``model_type`` segment entirely), or anything else that
+    doesn't start/end alphanumeric — would leave the row visible in the list
+    endpoint but permanently unreachable by get/update/delete/set-default,
+    surfacing as a spurious "not found". Percent-encoding never helps: ASGI
+    servers decode ``%2F``/dot-segment escapes before Starlette's router sees
+    the path.
+    """
     value = value.strip()
     if not value:
         raise ValueError("name must be non-empty")
+    if len(value) > _NAME_MAX_LENGTH:
+        raise ValueError(f"name must be at most {_NAME_MAX_LENGTH} characters")
+    if not _NAME_PATTERN.fullmatch(value):
+        raise ValueError(
+            "name must start and end with a letter or digit, and contain only "
+            "letters, digits, '.', '_', or '-' (it is used as a URL path segment)"
+        )
     return value
 
 

openrag/di/container.py

@@ -437,6 +437,7 @@ def model_endpoint_service(self) -> ModelEndpointService:
                 model_endpoint_repo=self.model_endpoint_repo,
                 config=self._require_settings(),
                 partition_service=self.partition_service,
+                preset_service=self.preset_service,
                 client_caches={
                     "embedder": self._embedder_cache,
                     "reranker": self._reranker_cache,

openrag/services/orchestrators/model_endpoint_service.py

@@ -114,11 +114,13 @@ def __init__(
         model_endpoint_repo: ModelEndpointRepository,
         config: Settings,
         partition_service: Any = None,
+        preset_service: Any = None,
         client_caches: dict[str, dict[str, Any]] | None = None,
     ) -> None:
         self._repo = model_endpoint_repo
         self._config = config
         self._partition_service = partition_service
+        self._preset_service = preset_service
         self._client_caches: dict[str, dict[str, Any]] = client_caches or {}
 
     # ------------------------------------------------------------------
@@ -396,6 +398,29 @@ async def update_model_endpoint(self, name: str, model_type: str, **fields: obje
         Pass ``new_name=`` to rename. After any change the in-memory config is
         reloaded and the stale cached client instance is evicted so the next
         request builds a fresh client against the updated config.
+
+        A rename also cascades to every stored reference — ``partitions.embedder``
+        / ``partitions.chat_llm`` and endpoint-name fields embedded in
+        ``pipeline_presets.config`` — inside the repo's own rename transaction
+        (see ``PgModelEndpointRepository.rename``, #770). Those writes are
+        invisible until the referencing services reload their in-memory caches,
+        which is why a rename also refreshes presets then partitions here —
+        the same order ``PresetService.update_preset`` uses, since partition
+        resolution reads the presets dict.
+
+        Both reload calls ``await``, so a concurrent request can run between
+        them — and the DB rename has *already* committed by that point. Without
+        ``_alias_renamed_name``, a request landing in that window could resolve
+        a partition/preset that the cascade already repointed at ``new_name``
+        against a registry that (until the final ``load_all()`` below) still
+        only knows ``name`` — a bare ``KeyError``. The alias makes both ``name``
+        and ``new_name`` resolve immediately, built from the row this call just
+        wrote — not whatever the in-memory bucket held before it — so a rename
+        combined with a field change (e.g. a new ``endpoint``) aliases the
+        *updated* config, not a stale pre-update one. That also covers a reload
+        call above raising: the registry stays queryable under both names,
+        correctly, instead of the update's failure leaving it stuck on a stale
+        config until process restart.
         """
         existing = await self._repo.get(name, model_type)
         if existing is None:
@@ -423,6 +448,21 @@ async def update_model_endpoint(self, name: str, model_type: str, **fields: obje
             await self._repo.rename(name, model_type, new_name)
             effective_name = new_name
             renamed_from = name
+            self._alias_renamed_name(model_type, name, new_name, updated or existing)
+            # A cached *client instance* under either name would otherwise survive
+            # this alias — the factory checks its cache before consulting the
+            # config registry, so a stale pre-rename/pre-update client would keep
+            # serving until the eviction at the end of this method, which a
+            # reload call below raising would skip entirely. The config alias
+            # above is already fresh, so evicting now is safe: anything rebuilt
+            # from either name resolves through the up-to-date config, not stale
+            # cached state.
+            self._invalidate_client_cache(model_type, name)
+            self._invalidate_client_cache(model_type, new_name)
+            if self._preset_service is not None:
+                await self._preset_service.load_all()
+            if self._partition_service is not None:
+                await self._partition_service.load_partitions()
 
         if promote_to_default:
             # Clears any prior default and sets this row in one transaction, then
@@ -540,6 +580,39 @@ async def validate_endpoint(
     # Internals
     # ------------------------------------------------------------------
 
+    def _alias_renamed_name(self, model_type: str, old_name: str, new_name: str, row: ModelEndpointRow) -> None:
+        """Make both ``old_name`` and ``new_name`` resolve to *row* before any reload runs.
+
+        Runs synchronously right after the rename ``await`` returns — no
+        further ``await`` happens before this executes, so no concurrent
+        request can observe the DB already renamed while the registry still
+        only answers to ``old_name``.
+
+        Built from ``row`` — the just-written DB state — rather than copying
+        whatever the in-memory bucket currently holds under ``old_name``: a
+        rename can land in the same call as a field update (e.g. a new
+        ``endpoint`` URL), applied to the DB *before* this runs, so the stale
+        in-memory entry would alias both names to the pre-update config. If a
+        reload below then raises, that staleness would never get corrected
+        by the final ``load_all()`` this call never reaches — the registry
+        would keep serving the old endpoint under the new (DB-authoritative)
+        name until process restart. The next full ``load_all()`` (below, or
+        from any later CRUD call) rebuilds the bucket straight from DB and
+        drops the ``old_name`` entry on its own.
+        """
+        bucket: dict[str, Any] | None = getattr(self._config.models, model_type, None)
+        if bucket is None:
+            return
+        cfg = ModelEndpointConfig(
+            endpoint=row.endpoint,
+            model_name=row.model_name,
+            batch_size=row.batch_size,
+            timeout=row.timeout,
+            extra=row.extra,
+        )
+        bucket[old_name] = cfg
+        bucket[new_name] = cfg
+
     def _invalidate_client_cache(self, model_type: str, name: str) -> None:
         """Evict ``name`` from the component-factory cache for ``model_type``."""
         cache = self._client_caches.get(model_type)

openrag/services/orchestrators/retrieval_service.py

@@ -164,6 +164,71 @@ def _require_partition_config(self, partition: str):
     def _legacy_retriever_value(self, name: str, default: Any) -> Any:
         return getattr(self._config.retriever, name, default)
 
+    def _resolve_reranker(self, reranker_name: str | None, partition: str) -> Reranker | None:
+        """Effective reranker for one partition's retrieval pipeline.
+
+        Resolution order — mirrors ``QueryService._resolve_llm``:
+
+        1. The partition's configured ``reranker`` preset, resolved fresh via
+           the model-endpoint catalog factory so a rename/promotion of that
+           endpoint takes effect immediately.
+        2. The **catalog default** endpoint (``is_default=True``) when the
+           partition sets no preset, or its preset name has gone stale (the
+           endpoint was renamed/deleted after assignment — unlike
+           ``chat_llm``, this field has no create/PATCH-time validation, so a
+           stale name reaching here is expected, not a bug).
+        3. The static reranker built at startup from ``settings.reranker``,
+           only when no factory is wired (unit tests) or the catalog has no
+           default reranker endpoint yet.
+
+        The resolved endpoint name is always logged (at debug), including for
+        the default, so "which reranker ran?" is answerable from the logs.
+        """
+        if self._reranker_factory is None:
+            return self._legacy_reranker
+        if reranker_name:
+            try:
+                reranker = self._reranker_factory(reranker_name)
+            except KeyError:
+                logger.bind(reranker=reranker_name, partition=partition).warning(
+                    "Partition reranker preset not found in the model-endpoint catalog — "
+                    "falling back to the default reranker"
+                )
+            else:
+                logger.bind(reranker=reranker_name, partition=partition).debug(
+                    "Reranking with the partition's reranker preset"
+                )
+                return reranker
+        try:
+            reranker = self._reranker_factory("default")
+        except KeyError:
+            pass
+        else:
+            logger.bind(reranker=self._default_reranker_name(), partition=partition).debug(
+                "Reranking with the default reranker preset"
+            )
+            return reranker
+        logger.bind(partition=partition).debug(
+            "Reranking with the static default reranker (no catalog default endpoint)"
+        )
+        return self._legacy_reranker
+
+    def _default_reranker_name(self) -> str:
+        """Real endpoint name behind the catalog reranker ``"default"`` alias, for logging.
+
+        Same identity-lookup trick as ``QueryService._default_llm_name``: the
+        ``"default"`` alias config object is the *same* object as its real-named
+        entry, so the name is recovered by identity. Returns ``"default"`` when
+        it can't be resolved (e.g. the alias isn't populated yet).
+        """
+        rerankers = self._config.models.reranker
+        default_cfg = rerankers.get("default")
+        if default_cfg is not None:
+            for name, cfg in rerankers.items():
+                if name != "default" and cfg is default_cfg:
+                    return name
+        return "default"
+
     def _pipeline_for_partition(self, partition: str) -> tuple[RetrieverPipeline, int | None]:
         # Callers only ever pass a concrete partition name — the "all" sentinel is
         # expanded to concrete keys by _pipeline_groups_for_partitions before this
@@ -182,12 +247,7 @@ def _pipeline_for_partition(self, partition: str) -> tuple[RetrieverPipeline, in
         if rtype in {"multiQuery", "hyde"} and self._llm_factory is not None:
             llm = self._llm_factory(pipeline_cfg.llm or partition_cfg.chat_llm or "default")
 
-        reranker = None
-        if pipeline_cfg.enable_reranker:
-            if self._reranker_factory is not None:
-                reranker = self._reranker_factory(pipeline_cfg.reranker or "default")
-            else:
-                reranker = self._legacy_reranker
+        reranker = self._resolve_reranker(pipeline_cfg.reranker, partition) if pipeline_cfg.enable_reranker else None
 
         retriever = self._build_retriever(
             rtype=rtype,

openrag/services/persistence/model_endpoint_repo.py

@@ -27,6 +27,20 @@
 # transaction; ModelEndpointService.update_model_endpoint routes is_default there.
 _ALLOWED_UPDATE_FIELDS = frozenset({"endpoint", "model_name", "batch_size", "timeout", "extra"})
 
+# Endpoint names are referenced by value elsewhere, and nothing updates those
+# references when an endpoint is renamed (#770) — so ``rename()`` cascades to
+# every known reference in the same transaction as the name change. Direct
+# endpoint-name columns on ``partitions``, keyed by the model_type they hold:
+_PARTITION_COLUMN_BY_TYPE = {"embedder": "embedder", "llm": "chat_llm"}
+# Endpoint-name keys embedded in ``pipeline_presets.config`` (JSONB), by the
+# preset_type that carries them — see core/config/retrieval_pipeline.py and
+# core/config/indexation_pipeline.py for the field definitions.
+_RETRIEVAL_PRESET_KEYS_BY_TYPE = {"llm": ("llm",), "reranker": ("reranker",)}
+_INDEXATION_PRESET_KEYS_BY_TYPE = {
+    "llm": ("contextualization_llm", "metadata_extraction_llm", "topic_tagging_llm"),
+    "vlm": ("vlm",),
+}
+
 
 class PgModelEndpointRepository(ModelEndpointRepository):
     """asyncpg-backed implementation of :class:`ModelEndpointRepository`."""
@@ -124,12 +138,80 @@ async def update(self, name: str, model_type: str, **fields: object) -> ModelEnd
         return self._to_model(rec) if rec else None
 
     async def rename(self, name: str, model_type: str, new_name: str) -> None:
-        await self.pool.execute(
-            "UPDATE model_endpoints SET name = $3, updated_at = now() WHERE name = $1 AND model_type = $2",
-            name,
-            model_type,
-            new_name,
-        )
+        """Rename an endpoint and cascade the new name to every stored reference.
+
+        Left alone, a rename silently strands every partition or preset that
+        pointed at the old name — ``partitions.embedder`` / ``partitions.chat_llm``,
+        and the endpoint-name fields embedded in ``pipeline_presets.config``
+        (JSONB) — since nothing else in the schema updates those when the
+        referenced row's name changes (#770). All writes run in this one
+        transaction so a partial cascade can never leave the registry and its
+        referents disagreeing.
+
+        The caller (``ModelEndpointService.update_model_endpoint``) still owns
+        refreshing the in-memory partition/preset caches afterwards — this
+        method only makes the DB-side references consistent.
+
+        Raises :class:`NotFoundError` if ``name`` vanished between the
+        service's existence check and this transaction (a concurrent delete)
+        — mirroring ``PgPipelinePresetRepository.rename``. Without the
+        ``RETURNING`` check, a lost race would still run the cascade below,
+        repointing partitions/presets at a ``new_name`` that was never
+        actually created.
+
+        Also ``LOCK``s ``partitions`` ``IN SHARE MODE`` before touching
+        anything — the same lock :meth:`PgPresetRepository.delete` takes, and
+        the same table :meth:`PgPartitionRepository.update_partition` writes
+        to *before* its own DB-authoritative ``chat_llm`` re-check. Without
+        this, a partition PATCH could validate ``name`` against the in-memory
+        catalog, then block behind this transaction's cascade on the exact
+        row it's about to write, and resume writing the now-renamed-away
+        ``name`` straight back once this commits — permanently stranding that
+        partition the moment the service's temporary alias (see
+        ``ModelEndpointService._alias_renamed_name``) drops. Locking first, in
+        the same order both call sites use, makes the two block on each other
+        instead of interleaving: whichever transaction's ``partitions`` write
+        commits first is the one the other observes.
+        """
+        async with self.pool.acquire() as conn:
+            async with conn.transaction():
+                await conn.execute("LOCK TABLE partitions IN SHARE MODE")
+
+                renamed = await conn.fetchrow(
+                    "UPDATE model_endpoints SET name = $3, updated_at = now() "
+                    "WHERE name = $1 AND model_type = $2 RETURNING name",
+                    name,
+                    model_type,
+                    new_name,
+                )
+                if renamed is None:
+                    raise NotFoundError(f"Endpoint '{name}' of type '{model_type}' not found.")
+
+                partition_col = _PARTITION_COLUMN_BY_TYPE.get(model_type)
+                if partition_col:
+                    await conn.execute(
+                        f"UPDATE partitions SET {partition_col} = $2 WHERE {partition_col} = $1",
+                        name,
+                        new_name,
+                    )
+
+                for preset_type, keys in (
+                    ("retrieval", _RETRIEVAL_PRESET_KEYS_BY_TYPE.get(model_type, ())),
+                    ("indexation", _INDEXATION_PRESET_KEYS_BY_TYPE.get(model_type, ())),
+                ):
+                    for key in keys:
+                        await conn.execute(
+                            """
+                            UPDATE pipeline_presets
+                            SET config = jsonb_set(config, $1::text[], to_jsonb($2::text)), updated_at = now()
+                            WHERE preset_type = $3 AND config->>$4 = $5
+                            """,
+                            [key],
+                            new_name,
+                            preset_type,
+                            key,
+                            name,
+                        )
 
     async def delete(self, name: str, model_type: str) -> bool:
         result = await self.pool.execute(