fix(review): scope registry review queries to the caller's org by Nav-Prak · Pull Request #1288 · Observal/Observal

Nav-Prak · 2026-05-30T20:04:49Z

Purpose / Description

Constrain registry review access to the reviewer's organization so a reviewer in one org cannot list, read, approve, reject, or bulk-act on another org's submissions.

This is part of the AUDIT2 tenant-scoping work and is split out from the closed broader tenant-scope PR #1184 to keep the change smaller and independently reviewable.

Fixes

Refs Fix tenant scoping for review, eval, audit, and security events #1184
Addresses AUDIT2 tenant-scoping for registry review paths

Approach

This change scopes review-queue listing, agent, component, and bundle lookups to the caller's owner_org_id.

Adds _owner_org_conditions and _apply_owner_org_filter helpers.
Threads org_id through _find_listing, _check_agent_components_ready, _query_pending_agents, _query_pending_components, and review mutation handlers.
Applies org scoping to list/detail/approve/reject, agent approve/reject, bundle approve/reject, related-skills, and approve-with-skills paths.
Treats components the caller's org cannot see as blocking rather than silently approvable.
Preserves the existing org-less/super-admin behavior by omitting the org predicate when the caller has no org.

How Has This Been Tested?

Tested the new review tenant-scope coverage and linted/formatted the changed files.

Targeted tests:

cd observal-server && uv run --with pytest --with pytest-asyncio --with pyyaml \
  --with typer --with rich --with hypothesis --with pyarrow \
  pytest ../tests/test_tenant_scope_review.py -q

Result: 18 review tenant-scope tests passed.

Lint/format:

uv run --with ruff==0.15.10 ruff check observal-server/api/routes/review.py ../tests/test_tenant_scope_review.py
uv run --with ruff==0.15.10 ruff format --check observal-server/api/routes/review.py ../tests/test_tenant_scope_review.py

Also re-ran adjacent review/agent/bundle/draft suites as part of the split verification; no regressions were found.

Checklist

You have a descriptive commit message with a short title (first line, max 50 chars).
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code
UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)

Summary by CodeRabbit

Bug Fixes
- Review listings and actions now remain isolated to the reviewer’s organization.
- Cross-organization components, bundles, skills, approvals, and rejections are blocked.
- Unauthorized review actions no longer commit changes.
- Organization-less super-admin access remains supported.

codecov · 2026-06-01T00:04:57Z

Codecov Report

❌ Patch coverage is 85.91549% with 10 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
observal-server/api/routes/review.py	85.91%	10 Missing ⚠️

📢 Thoughts on this report? Let us know!

Constrain review-queue listing, agent, component, and bundle lookups to the reviewer's organization (owner_org_id) so a reviewer in one org cannot read or act on another org's submissions. - Add _owner_org_conditions/_apply_owner_org_filter helpers and thread org_id through _find_listing, _check_agent_components_ready, _query_pending_agents, _query_pending_components, and the approve/ reject/bundle/related-skills handlers. - Treat components the caller's org cannot see as blocking rather than silently approvable. - Add tenant-scope regression tests: helper-level coverage plus route- level tests that drive each handler (list, detail, approve/reject, agent approve/reject, bundle approve/reject, related-skills, approve- with-skills) and assert cross-org access is denied (404/empty) while the org predicate is present in the query (and absent for super-admins). Part of AUDIT2 tenant-scoping, split out of Observal#1184.

coderabbitai · 2026-07-31T17:41:52Z

Walkthrough

Review routes now enforce organization-level isolation for listing, agent, component, bundle, related-skill, approval, and rejection operations. Tenant-scoping tests cover cross-organization denial, organization propagation, super-admin access, and commit prevention.

Changes

Tenant-scoped review

Layer / File(s)	Summary
Scoped lookups and pending review queue `observal-server/api/routes/review.py`, `tests/test_tenant_scope_review.py`	Listing resolution, readiness checks, and pending agent or component queries apply the reviewer’s `org_id`.
Scoped review retrieval `observal-server/api/routes/review.py`, `tests/test_tenant_scope_review.py`	Review-detail agent and component lookups restrict results to the reviewer’s organization.
Scoped approval and rejection `observal-server/api/routes/review.py`, `tests/test_tenant_scope_review.py`	Listing, agent, and bundle actions validate organization ownership and reject cross-organization requests without committing.
Scoped related-skill actions `observal-server/api/routes/review.py`, `tests/test_tenant_scope_review.py`	Related-skill retrieval and bulk approval lookups restrict listings and skills to the reviewer’s organization.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

#1094 — The changes implement organization-scoped review queues, actions, and tenant-isolation tests.

Suggested reviewers: haz3-jolt

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.52% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely states that registry review queries are scoped to the caller's organization.
Description check	✅ Passed	The description covers the purpose, approach, testing, checklist, and tenant-scoping behavior; only optional or non-critical template details are omitted.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

Fix failing CI checks

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

observal-server/api/routes/review.py (1)
521-524: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse _apply_owner_org_filter instead of duplicating the Agent org check. get_review, approve_agent, and reject_agent each repeat the same inline if current_user.org_id is not None: ...where(Agent.owner_org_id == current_user.org_id) pattern, even though this PR adds _apply_owner_org_filter for exactly this purpose and already unit-tests it. Consolidating removes duplicated logic that could otherwise diverge.

observal-server/api/routes/review.py#L521-L524: replace the inline if current_user.org_id is not None block with agent_stmt = _apply_owner_org_filter(select(Agent).where(Agent.id == agent_uuid), Agent, current_user.org_id).

observal-server/api/routes/review.py#L727-L730: replace the inline block with agent_stmt = _apply_owner_org_filter(select(Agent).where(Agent.id == agent_id), Agent, current_user.org_id).

observal-server/api/routes/review.py#L802-L805: replace the inline block with agent_stmt = _apply_owner_org_filter(select(Agent).where(Agent.id == agent_id), Agent, current_user.org_id).
♻️ Proposed refactor (apply the same pattern at all three sites)
-        agent_stmt = select(Agent).where(Agent.id == agent_uuid)
-        if current_user.org_id is not None:
-            agent_stmt = agent_stmt.where(Agent.owner_org_id == current_user.org_id)
-        agent = (await db.execute(agent_stmt)).scalar_one_or_none()
+        agent_stmt = _apply_owner_org_filter(
+            select(Agent).where(Agent.id == agent_uuid), Agent, current_user.org_id
+        )
+        agent = (await db.execute(agent_stmt)).scalar_one_or_none()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@observal-server/api/routes/review.py` around lines 521 - 524, Replace the
duplicated inline Agent organization checks with _apply_owner_org_filter in
get_review (observal-server/api/routes/review.py:521-524), approve_agent
(observal-server/api/routes/review.py:727-730), and reject_agent
(observal-server/api/routes/review.py:802-805), passing each existing Agent
query, Agent, and current_user.org_id.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@observal-server/api/routes/review.py`:
- Around line 521-524: Replace the duplicated inline Agent organization checks
with _apply_owner_org_filter in get_review
(observal-server/api/routes/review.py:521-524), approve_agent
(observal-server/api/routes/review.py:727-730), and reject_agent
(observal-server/api/routes/review.py:802-805), passing each existing Agent
query, Agent, and current_user.org_id.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 740058ad-7b0e-4bfe-9be9-a5ca1e53ab42

📥 Commits

Reviewing files that changed from the base of the PR and between 0552157 and 3bc4c60.

📒 Files selected for processing (2)

observal-server/api/routes/review.py
tests/test_tenant_scope_review.py

github-actions Bot added new contributor Pull request from a first-time contributor server Pull request touches server code tests Pull request adds or modifies tests labels May 30, 2026

Haz3-jolt force-pushed the main branch from 0421b87 to 3e1a61b Compare July 16, 2026 14:17

Haz3-jolt force-pushed the pr/audit2-review-org-scope branch from 13749b2 to 3bc4c60 Compare July 31, 2026 17:41

coderabbitai Bot reviewed Jul 31, 2026

View reviewed changes

This was referenced Aug 2, 2026

feat(registry): add team publishing and team-private visibility #1640

Merged

refactor(scope): remove legacy tenant scaffold #1641

Merged

ShaanNarendran closed this Sep 13, 2026

Navigation Menu

fix(review): scope registry review queries to the caller's org - #1288

fix(review): scope registry review queries to the caller's org#1288
Nav-Prak wants to merge 1 commit into
Observal:mainfrom
Nav-Prak:pr/audit2-review-org-scope

Nav-Prak commented May 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

codecov Bot commented Jun 1, 2026

Uh oh!

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Nav-Prak commented May 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Checklist

Summary by CodeRabbit

Uh oh!

codecov Bot commented Jun 1, 2026

Codecov Report

Uh oh!

coderabbitai Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Nav-Prak commented May 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading