feat(agents): collapse denied agent route access to 404 instead of 403 by asherfink · Pull Request #271 · scaleapi/scale-agentex

asherfink · 2026-06-03T15:17:06Z

What

Direct agent routes now return 404 instead of 403 when the caller isn't allowed to see the agent.

Why

A 403 on a specific agent id or name leaks that the agent exists. Collapsing denials to 404 means a caller can't tell "exists but you can't access it" from "doesn't exist", so they can't probe for agents living in other tenants.

Changes

DAuthorizedId(agent, ...) and DAuthorizedName(agent, ...) collapse an authorization denial to a 404, matching the existing behavior for tasks and api keys.
Because the RPC-by-id and RPC-by-name routes go through these same dependencies, they collapse too — not just plain reads and deletes.
The name variant resolves name to id first, so a genuinely missing name still surfaces the normal repository 404, and only a denial on a resolved id is collapsed.
Adds a small _check_agent_or_collapse_to_404 helper next to the existing task and api-key collapse helpers. Folding the three into one generic helper is a tracked follow-up.
The list endpoint is unaffected: it already filters down to the authorized id set.

Tests

Unit tests for the agent collapse helper and the id/name dependencies: allowed passes through, denied collapses to 404, an absent name surfaces the native 404, and the operation is forwarded verbatim.
Updated a tasks-authz test that previously asserted agent name-routes returned 403; they now collapse to 404 like tasks.

Stacking

Stacked on #270 (agent ownership writes). Review that one first.

Ticket: AGX1-242

Greptile Summary

This PR collapses denied agent route access from 403 to 404, preventing callers from probing cross-tenant agent existence by distinguishing "exists but forbidden" from "not found." The helper _check_agent_or_collapse_to_404 is wired into DAuthorizedId and DAuthorizedName, mirroring the existing task and API-key collapse patterns.

DAuthorizedId and DAuthorizedName: both now route AgentexResourceType.agent through the new collapse helper, covering path-parameter reads, deletes, and RPC-by-name routes.
DAuthorizedQuery gap: the GET /events?agent_id= list endpoint uses DAuthorizedQuery(AgentexResourceType.agent, ...), which has no agent branch and still falls through to authorization.check(...) — returning 403 on denial and leaking existence. This was flagged in the previous review iteration and remains unaddressed.
Tests: a new test_agents_authz.py covers the collapse helper and both dependency wrappers; test_tasks_authz.py is updated to expect 404 for the agent name path.

Confidence Score: 4/5

Safe to merge after addressing the DAuthorizedQuery gap; the list_events agent_id query path still returns 403 on denial.

The collapse logic is correctly applied to DAuthorizedId and DAuthorizedName, but DAuthorizedQuery has no agent branch — the GET /events?agent_id= endpoint calls authorization.check directly and returns 403 when the caller is denied, leaking that the agent ID exists in another tenant.

agentex/src/utils/authorization_shortcuts.py — the DAuthorizedQuery function (lines 118–157) needs an elif agent branch matching what was added to DAuthorizedId and DAuthorizedName.

Important Files Changed

Filename	Overview
agentex/src/utils/authorization_shortcuts.py	Adds _check_agent_or_collapse_to_404 and wires it into DAuthorizedId and DAuthorizedName, but DAuthorizedQuery is not updated — the GET /events?agent_id= path still returns 403 on denial, leaking cross-tenant existence.
agentex/tests/unit/api/test_agents_authz.py	New test file covering the collapse helper, DAuthorizedId, DAuthorizedName, and list filtering; all relevant allowed/denied/absent cases are exercised.
agentex/tests/unit/api/test_tasks_authz.py	Updates one test that previously expected 403 on agent name denial to now expect 404, correctly reflecting the new collapse behaviour.

Comments Outside Diff (2)

agentex/src/utils/authorization_shortcuts.py, line 118-157 (link)

DAuthorizedQuery still returns 403 for denied agent access

DAuthorizedQuery is used with AgentexResourceType.agent on the GET /events endpoint (events.py lines 55–60). That path falls through to the unmodified else branch (authorization.check), so a 403 is still returned when the caller is denied — leaking that the agent ID exists. The fix applied to DAuthorizedId and DAuthorizedName needs to be mirrored here: add an elif resource_type == AgentexResourceType.agent branch that calls _check_agent_or_collapse_to_404, just as was done in DAuthorizedId.

Prompt To Fix With AI

This is a comment left during a code review.
Path: agentex/src/utils/authorization_shortcuts.py
Line: 118-157

Comment:
**`DAuthorizedQuery` still returns 403 for denied agent access**

`DAuthorizedQuery` is used with `AgentexResourceType.agent` on the `GET /events` endpoint (`events.py` lines 55–60). That path falls through to the unmodified `else` branch (`authorization.check`), so a 403 is still returned when the caller is denied — leaking that the agent ID exists. The fix applied to `DAuthorizedId` and `DAuthorizedName` needs to be mirrored here: add an `elif resource_type == AgentexResourceType.agent` branch that calls `_check_agent_or_collapse_to_404`, just as was done in `DAuthorizedId`.

How can I resolve this? If you propose a fix, please make it concise.

agentex/src/utils/authorization_shortcuts.py, line 118-157 (link)

DAuthorizedQuery still returns 403 for denied agent access

DAuthorizedQuery is used with AgentexResourceType.agent on the GET /events list endpoint (events.py lines 55–60). Its else branch calls authorization.check(...) directly, so a denied agent check raises AuthorizationError (403) instead of ItemDoesNotExist (404) — leaking that the agent ID exists across tenants. The fix applied to DAuthorizedId and DAuthorizedName in this PR needs a matching elif resource_type == AgentexResourceType.agent branch added to DAuthorizedQuery as well.

Prompt To Fix With AI

This is a comment left during a code review.
Path: agentex/src/utils/authorization_shortcuts.py
Line: 118-157

Comment:
**`DAuthorizedQuery` still returns 403 for denied agent access**

`DAuthorizedQuery` is used with `AgentexResourceType.agent` on the `GET /events` list endpoint (`events.py` lines 55–60). Its `else` branch calls `authorization.check(...)` directly, so a denied agent check raises `AuthorizationError` (403) instead of `ItemDoesNotExist` (404) — leaking that the agent ID exists across tenants. The fix applied to `DAuthorizedId` and `DAuthorizedName` in this PR needs a matching `elif resource_type == AgentexResourceType.agent` branch added to `DAuthorizedQuery` as well.

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
agentex/src/utils/authorization_shortcuts.py:118-157
**`DAuthorizedQuery` still returns 403 for denied agent access**

`DAuthorizedQuery` is used with `AgentexResourceType.agent` on the `GET /events` list endpoint (`events.py` lines 55–60). Its `else` branch calls `authorization.check(...)` directly, so a denied agent check raises `AuthorizationError` (403) instead of `ItemDoesNotExist` (404) — leaking that the agent ID exists across tenants. The fix applied to `DAuthorizedId` and `DAuthorizedName` in this PR needs a matching `elif resource_type == AgentexResourceType.agent` branch added to `DAuthorizedQuery` as well.

_{Reviews (2): Last reviewed commit: "feat(agents): collapse denied agent rout..." | Re-trigger Greptile}

Direct agent-by-id and agent-by-name routes now return 404 instead of 403 when the caller isn't authorized for the agent, matching the existing behavior for tasks and api keys. A 403 on a specific id or name reveals that the agent exists, so collapsing denials to 404 stops callers from probing for agents in other tenants. The name routes resolve the name to an id first, so a genuinely missing name still surfaces the normal repository 404. Ticket: AGX1-242

asherfink requested a review from a team as a code owner June 3, 2026 15:17

asherfink mentioned this pull request Jun 3, 2026

feat(agents): write agent ownership to the authorization graph on create/delete #270

Open

asherfink force-pushed the asher.fink/agx1-242-agent-dual-write branch from 685126a to 0d99f31 Compare June 3, 2026 15:31

asherfink force-pushed the asher.fink/agx1-242-agent-route-fgac branch from e358194 to eec235f Compare June 3, 2026 15:31

asherfink changed the title ~~feat(agents): collapse denied agent reads to 404 on direct routes~~ feat(agents): collapse denied agent route access to 404 instead of 403 Jun 3, 2026

asherfink force-pushed the asher.fink/agx1-242-agent-route-fgac branch from eec235f to 0cfb83c Compare June 3, 2026 15:35

asherfink force-pushed the asher.fink/agx1-242-agent-dual-write branch from 0d99f31 to 14796e9 Compare June 3, 2026 15:35

jenniechung approved these changes Jun 3, 2026

View reviewed changes

harvhan approved these changes Jun 4, 2026

View reviewed changes

asherfink force-pushed the asher.fink/agx1-242-agent-route-fgac branch from 0cfb83c to ec8c2ec Compare June 5, 2026 17:07

asherfink force-pushed the asher.fink/agx1-242-agent-dual-write branch from 14796e9 to 883921e Compare June 5, 2026 17:07

asherfink force-pushed the asher.fink/agx1-242-agent-dual-write branch from 883921e to 6de8f39 Compare June 5, 2026 17:29

asherfink force-pushed the asher.fink/agx1-242-agent-route-fgac branch from ec8c2ec to f7e134b Compare June 5, 2026 17:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(agents): collapse denied agent route access to 404 instead of 403#271

feat(agents): collapse denied agent route access to 404 instead of 403#271
asherfink wants to merge 1 commit into
asher.fink/agx1-242-agent-dual-writefrom
asher.fink/agx1-242-agent-route-fgac

asherfink commented Jun 3, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

asherfink commented Jun 3, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Changes

Tests

Stacking

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Comments Outside Diff (2)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

asherfink commented Jun 3, 2026 •

edited by greptile-apps Bot

Loading