fix(indexing): return 403 (not 500) when a create-race loser lacks access - #577
fix(indexing): return 403 (not 500) when a create-race loser lacks access#577Ahmath-Gadji wants to merge 1 commit into
Conversation
…cess 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) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 28 minutes and 5 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 participants
What
Follow-up to a review finding on #568 (F11). The partition auto-create race guard (
_ensure_editor_access_after_create_raceinindexing_service.py) correctly fails closed when a concurrent create lost the race and the loser turns out not to be a member of the partition — but it raised a builtinPermissionError.Why it matters
The global exception handlers (
api/error_handlers.py) mapOpenRAGErrorsubclasses to their declared status and everything else to the catch-all 500. So a builtinPermissionErrorfrom this guard surfaced a legitimate authorization denial as an Internal Server Error instead of a403 Forbidden.This only triggers on the narrow cross-user race (two different users creating the same brand-new partition name simultaneously, where the loser has no membership). It already fails closed — this just corrects the status code. The common case (same user, concurrent uploads — the scenario the race guard was added for) is unaffected: the user owns the partition and the upload continues.
Change
indexing_service.py: raiseAuthError(status 403) instead ofPermissionError.test_indexing_service.py: the cross-user race test now expectsAuthErrorand assertsstatus_code == 403.Verification
uv run pytest tests/unit/services/orchestrators/test_indexing_service.py→ 18 passedruff checkclean🤖 Generated with Claude Code