From 8fe27798f0fdd2151597e61b53c8546f417ea1ca Mon Sep 17 00:00:00 2001 From: EnjoyBacon7 <59032058+EnjoyBacon7@users.noreply.github.com> Date: Thu, 25 Jun 2026 13:44:23 +0000 Subject: [PATCH] fix(indexing): return 403 (not 500) when a create-race loser lacks access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The partition auto-create race guard added for #568 re-verifies editor membership after a concurrent create lost the race. It raised a builtin `PermissionError`, which the global exception handlers don't map (only `OpenRAGError` subclasses get their declared status) — so the catch-all turned a legitimate authorization denial into an HTTP 500. Raise `AuthError` (status 403) instead, so a cross-user create race that the loser isn't a member of returns a clean Forbidden. Same-user concurrent uploads are unaffected (they own the partition and continue). Test asserts the raised error carries `status_code == 403`. Co-Authored-By: Claude Opus 4.8 (1M context) --- openrag/services/orchestrators/indexing_service.py | 7 +++++-- tests/unit/services/orchestrators/test_indexing_service.py | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/openrag/services/orchestrators/indexing_service.py b/openrag/services/orchestrators/indexing_service.py index a188aa170..f5d80226d 100644 --- a/openrag/services/orchestrators/indexing_service.py +++ b/openrag/services/orchestrators/indexing_service.py @@ -16,7 +16,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Any -from core.utils.exceptions import PartitionNotFoundError, ValidationError +from core.utils.exceptions import AuthError, PartitionNotFoundError, ValidationError from core.utils.filename import extract_temporal_fields from core.utils.logging import get_logger from core.utils.partition_limits import max_partitions_for_user @@ -123,7 +123,10 @@ async def _ensure_editor_access_after_create_race(self, partition: str, user: di members = await self._partition_service.list_members(partition) membership = next((m for m in members if m.get("user_id") == user.get("id")), None) if membership is None or _ROLE_HIERARCHY.get(membership.get("role", ""), 0) < _ROLE_HIERARCHY["editor"]: - raise PermissionError(f"Editor role required for partition: {partition}") + # AuthError (status 403) so the global handler returns a clean + # Forbidden; a builtin PermissionError would fall through to the + # catch-all handler and surface as a 500. + raise AuthError(f"Editor role required for partition: {partition}") def _resolve_indexation_dispatch_config(self, partition: str) -> tuple[dict | None, str | None]: partitions = self._partition_configs() diff --git a/tests/unit/services/orchestrators/test_indexing_service.py b/tests/unit/services/orchestrators/test_indexing_service.py index 64a9a7c9c..f8664dfbd 100644 --- a/tests/unit/services/orchestrators/test_indexing_service.py +++ b/tests/unit/services/orchestrators/test_indexing_service.py @@ -6,7 +6,7 @@ from core.config.indexation_pipeline import IndexationPipelineConfig from core.config.retrieval_pipeline import RetrievalPipelineConfig from core.models.preset import PartitionConfig -from core.utils.exceptions import PartitionNotFoundError, ValidationError +from core.utils.exceptions import AuthError, PartitionNotFoundError, ValidationError from services.orchestrators.indexing_service import IndexingService @@ -418,7 +418,7 @@ async def test_add_file_rejects_partition_exists_race_without_membership(tmp_pat psvc = RaceLostPartitionService(config, grant_owner=False) svc = _service(disp=disp, config=config, partition_service=psvc) - with pytest.raises(PermissionError, match="Editor role required"): + with pytest.raises(AuthError, match="Editor role required") as exc_info: await svc.add_file( file_path=str(f), file_id="f1", @@ -429,6 +429,8 @@ async def test_add_file_rejects_partition_exists_race_without_membership(tmp_pat user={"id": 7}, ) + # Maps to a clean 403 Forbidden, not the catch-all 500. + assert exc_info.value.status_code == 403 assert disp.dispatched == []