From cc50f6541bdd2ed11f2c9b392363016fad6ec820 Mon Sep 17 00:00:00 2001 From: Harvey Han Date: Tue, 9 Jun 2026 00:54:49 -0700 Subject: [PATCH 1/2] fix(authz): mint agentex resource ownership on create under FGAC Two issues prevented resource ownership from being recorded when fine-grained access control is enforced: - /agents/register-build was matched by the /agents/register entry in WHITELISTED_ROUTES through a prefix (startswith) check, so it bypassed authentication. With no principal, the ownership grant/register for the newly built agent ran with a null principal and failed the create check. Use a boundary-aware match (exact, or a true sub-path under the route) so /agents/register stays whitelisted (pod self-registration) while /agents/register-build authenticates and carries the caller principal. - The _register_*_in_auth helpers resolved the creator via getattr(principal_context, "user_id"/"service_account_id"), but principal_context is an untyped dict, so both were always None and registration was skipped with "no creator resolvable" for agent, agent_api_key, and schedule. Read the fields from the dict (with an attribute-access fallback) so register_resource fires for the resource and its children. --- agentex/src/api/middleware_utils.py | 6 +++++- agentex/src/domain/services/schedule_service.py | 10 ++++++++-- .../src/domain/use_cases/agent_api_keys_use_case.py | 10 ++++++++-- agentex/src/domain/use_cases/agents_use_case.py | 12 ++++++++++-- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/agentex/src/api/middleware_utils.py b/agentex/src/api/middleware_utils.py index c884d22a..92ab4c22 100644 --- a/agentex/src/api/middleware_utils.py +++ b/agentex/src/api/middleware_utils.py @@ -43,8 +43,12 @@ def is_whitelisted_route( path: str, whitelisted_routes: set[str] = WHITELISTED_ROUTES ) -> bool: """Check if a route path is whitelisted (bypasses authentication).""" + # Boundary-aware prefix match: a whitelisted route only matches the route + # itself or a sub-path under it (route + "/..."). Plain startswith() would + # let "/agents/register" whitelist "/agents/register-build" too, which must + # stay authenticated so it can carry the caller's principal (owner grant). return path in whitelisted_routes or any( - path.startswith(route) for route in whitelisted_routes + path == route or path.startswith(route + "/") for route in whitelisted_routes ) diff --git a/agentex/src/domain/services/schedule_service.py b/agentex/src/domain/services/schedule_service.py index c6b3102d..8f5ae02f 100644 --- a/agentex/src/domain/services/schedule_service.py +++ b/agentex/src/domain/services/schedule_service.py @@ -132,8 +132,14 @@ async def _register_schedule_in_auth( identity is resolvable and registration is skipped. """ principal_context = self.authorization_service.principal_context - user_id = getattr(principal_context, "user_id", None) - service_account_id = getattr(principal_context, "service_account_id", None) + # principal_context is `Any` (a dict from /v1/authn), not a typed model, + # so getattr always yields None and silently skips the Spark register. + if isinstance(principal_context, dict): + user_id = principal_context.get("user_id") + service_account_id = principal_context.get("service_account_id") + else: + user_id = getattr(principal_context, "user_id", None) + service_account_id = getattr(principal_context, "service_account_id", None) if user_id is None and service_account_id is None: logger.warning( "Skipping auth registration for schedule: no creator resolvable", diff --git a/agentex/src/domain/use_cases/agent_api_keys_use_case.py b/agentex/src/domain/use_cases/agent_api_keys_use_case.py index d89d21be..faf10a2a 100644 --- a/agentex/src/domain/use_cases/agent_api_keys_use_case.py +++ b/agentex/src/domain/use_cases/agent_api_keys_use_case.py @@ -120,8 +120,14 @@ async def _register_api_key_in_auth( persisted api key that cannot be authorized. """ principal_context = self.authorization_service.principal_context - user_id = getattr(principal_context, "user_id", None) - service_account_id = getattr(principal_context, "service_account_id", None) + # principal_context is `Any` (a dict from /v1/authn), not a typed model, + # so getattr always yields None and silently skips the Spark register. + if isinstance(principal_context, dict): + user_id = principal_context.get("user_id") + service_account_id = principal_context.get("service_account_id") + else: + user_id = getattr(principal_context, "user_id", None) + service_account_id = getattr(principal_context, "service_account_id", None) if user_id is None and service_account_id is None: logger.warning( "Skipping auth registration for api_key: no creator resolvable", diff --git a/agentex/src/domain/use_cases/agents_use_case.py b/agentex/src/domain/use_cases/agents_use_case.py index 5853ae7d..de4b0c47 100644 --- a/agentex/src/domain/use_cases/agents_use_case.py +++ b/agentex/src/domain/use_cases/agents_use_case.py @@ -68,8 +68,16 @@ async def _register_in_auth(self, agent_id: str) -> bool: a resource it never registered. """ principal_context = self.authorization_service.principal_context - user_id = getattr(principal_context, "user_id", None) - service_account_id = getattr(principal_context, "service_account_id", None) + # principal_context is `Any` (a dict from /v1/authn), not a typed model, + # so attribute access via getattr always yields None and silently skips + # the Spark resource registration. Read from the dict (fall back to attr + # access for any object-shaped principal). + if isinstance(principal_context, dict): + user_id = principal_context.get("user_id") + service_account_id = principal_context.get("service_account_id") + else: + user_id = getattr(principal_context, "user_id", None) + service_account_id = getattr(principal_context, "service_account_id", None) if user_id is None and service_account_id is None: logger.warning( "Skipping authorization registration for agent: no creator resolvable", From 9b93b01293d96b201201a440878123e68edbdee9 Mon Sep 17 00:00:00 2001 From: Harvey Han Date: Tue, 9 Jun 2026 08:47:16 -0700 Subject: [PATCH 2/2] test(events): expect 404 for denied agent route access (align with #271) #271 made denied agent-route access collapse to 404 instead of 403 and updated the unit authz tests, but this events integration test still asserted 403 -- leaving it red on main and on any open PR. Align the expectation and name with the 404 convention. CI confirms the events route already returns 404 for a denied agent (the assertion failed with `404 == 403`); this only updates the test to match the intended behavior. --- .../tests/integration/api/events/test_events_authz_api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/agentex/tests/integration/api/events/test_events_authz_api.py b/agentex/tests/integration/api/events/test_events_authz_api.py index 37463856..024e7f50 100644 --- a/agentex/tests/integration/api/events/test_events_authz_api.py +++ b/agentex/tests/integration/api/events/test_events_authz_api.py @@ -248,7 +248,7 @@ async def test_list_events_authorized_returns_200( "src.domain.services.authorization_service.AuthorizationService.is_enabled", return_value=True, ) - async def test_list_events_unauthorized_agent_returns_403( + async def test_list_events_unauthorized_agent_returns_404( self, is_enabled_authorization_mock, is_enabled_mock, @@ -257,7 +257,7 @@ async def test_list_events_unauthorized_agent_returns_403( test_agent, test_task, ): - """Direct-resource denials surface as 403 (convention from #249/#255).""" + """Denied agent-route access collapses to 404 (convention from #271).""" with patch( "src.utils.http_request_handler.HttpRequestHandler.post_with_error_handling", side_effect=_mock_post_factory(deny_agent_ids={test_agent.id}), @@ -265,4 +265,4 @@ async def test_list_events_unauthorized_agent_returns_403( response = await isolated_client.get( f"/events?task_id={test_task.id}&agent_id={test_agent.id}" ) - assert response.status_code == 403 + assert response.status_code == 404