From 4ac446b7965947f0fb6fa9bb06d8ce2f3f060f4d Mon Sep 17 00:00:00 2001 From: Dhruv Madhok Date: Tue, 2 Jun 2026 14:25:33 -0700 Subject: [PATCH] refactor(events): inline agent collapse-to-404 check; drop one-off helper Per @rpatel-scale's review on #257: ``check_agent_or_collapse_to_404`` had exactly one caller (``GET /events/{id}``) and duplicated the shape of ``_check_api_key_or_collapse_to_404`` and the in-flight ``check_task_or_collapse_to_404`` from #249. Rather than ship a third per-resource helper, inline the 4-line try/except in ``events.py`` and delete ``agent_authorization.py`` outright. Wire behavior is unchanged: one ``authz.check`` on the parent agent; denial surfaces as 404 (``ItemDoesNotExist``) so callers can't probe cross-tenant existence by comparing 403 vs 404. When #249 lands, a follow-up can fold the (then-three) inline/named copies into a single generic ``check_or_collapse_to_404(authz, resource, op)`` with full context across tasks / api_keys / events. --- agentex/src/api/routes/events.py | 17 +++++++++++----- agentex/src/utils/agent_authorization.py | 26 ------------------------ 2 files changed, 12 insertions(+), 31 deletions(-) delete mode 100644 agentex/src/utils/agent_authorization.py diff --git a/agentex/src/api/routes/events.py b/agentex/src/api/routes/events.py index cbda563f..2cfe4fb0 100644 --- a/agentex/src/api/routes/events.py +++ b/agentex/src/api/routes/events.py @@ -1,13 +1,15 @@ from fastapi import APIRouter, Query +from src.adapters.authorization.exceptions import AuthorizationError +from src.adapters.crud_store.exceptions import ItemDoesNotExist from src.api.schemas.authorization_types import ( + AgentexResource, AgentexResourceType, AuthorizedOperationType, ) from src.api.schemas.events import Event from src.domain.services.authorization_service import DAuthorizationService from src.domain.use_cases.events_use_case import DEventUseCase -from src.utils.agent_authorization import check_agent_or_collapse_to_404 from src.utils.authorization_shortcuts import DAuthorizedQuery from src.utils.logging import make_logger @@ -25,11 +27,16 @@ async def get_event( event_use_case: DEventUseCase, authorization: DAuthorizationService, ) -> Event: - # Events delegate authz to the parent agent. + # Events delegate authz to the parent agent; collapse denial to 404 so + # callers can't probe cross-tenant existence by comparing 403 vs 404. event_entity = await event_use_case.get(event_id) - await check_agent_or_collapse_to_404( - authorization, event_entity.agent_id, AuthorizedOperationType.read - ) + try: + await authorization.check( + resource=AgentexResource.agent(event_entity.agent_id), + operation=AuthorizedOperationType.read, + ) + except AuthorizationError: + raise ItemDoesNotExist(f"Item with id '{event_id}' does not exist.") from None return Event.model_validate(event_entity) diff --git a/agentex/src/utils/agent_authorization.py b/agentex/src/utils/agent_authorization.py deleted file mode 100644 index 42dc6eb4..00000000 --- a/agentex/src/utils/agent_authorization.py +++ /dev/null @@ -1,26 +0,0 @@ -from src.adapters.authorization.exceptions import AuthorizationError -from src.adapters.crud_store.exceptions import ItemDoesNotExist -from src.api.schemas.authorization_types import ( - AgentexResource, - AuthorizedOperationType, -) - - -async def check_agent_or_collapse_to_404( - authorization, - agent_id: str, - operation: AuthorizedOperationType, -) -> None: - """Check an agent resource; collapse any denial to 404 to avoid leaking - cross-tenant existence. Mirrors ``check_task_or_collapse_to_404`` in - ``task_authorization.py`` — see that docstring for the full rationale. - - TODO: Restore the 403/404 split once agents carry tenant scope. - """ - try: - await authorization.check( - resource=AgentexResource.agent(agent_id), - operation=operation, - ) - except AuthorizationError: - raise ItemDoesNotExist(f"Item with id '{agent_id}' does not exist.") from None