feat(AGX1-277): enforce task.read on message GET via Spark AuthZ

[harvhan]

Linear Issue

AGX1-277 — Parent epic: AGX1-264.

Summary

Guard GET /messages/{message_id} (previously unauthorized) with a Spark AuthZ check on the parent task by resolving the message to its task_id and requiring task.read. Wires this through the existing TaskChildResourceType pattern — message joins event and state so the existing DAuthorizedId(...) machinery handles it.

Collapses task-child auth denials to 404 (ItemDoesNotExist) across path-id and query-id surfaces so callers cannot probe cross-tenant existence by comparing 403 vs 404.

Corrects the misleading TaskMessage | None return type on get_message (use case + route) — the underlying service already raised ItemDoesNotExist; the None branch was dead code.

Changes

• src/api/schemas/authorization_types.py — add message to TaskChildResourceType.
• src/utils/authorization_shortcuts.py — thread DTaskMessageRepository through _get_parent_task_id and inject message_repository into the _ensure_authorized_id / _ensure_authorized_query deps. Wrap the task-child check in try/except AuthorizationError → ItemDoesNotExist to collapse denial to 404 on both surfaces.
• src/api/routes/messages.py — add DAuthorizedId(TaskChildResourceType.message, AuthorizedOperationType.read, ...) to get_message; drop the dead else None branch and tighten the return type.
• src/domain/use_cases/messages_use_case.py — fix get_message return annotation from TaskMessageEntity | None to TaskMessageEntity.
• tests/integration/api/messages/test_messages_authz_api.py (new) — integration tests covering the auth matrix.

Behavior change beyond the ticket

The 404 collapse extends to event and state child routes as well, since the TaskChildResourceType branch is shared. The cross-tenant-leak argument is the same for those resources. Aligns with the direction AGX1-275 takes more broadly.

Out of scope

• POST/PUT message routes still surface 403 on deny. They use DAuthorizedBodyId(task, execute) and will be rewritten to task.update + 404-collapse by AGX1-275.
• RPC message/send similarly handled by AGX1-275.
• The agent-API-key path (request.state.agent_identity) still bypasses FGAC via AuthorizationService._bypass(). The ticket does not address this; it's a parent-epic-level gap and will be tracked separately under AGX1-264.

Coordination

Will conflict with AGX1-275 (#249) on the _ensure_authorized_id / _ensure_authorized_query bodies — that PR introduces a shared _check_task_or_collapse_to_404 helper in src/utils/task_authorization.py that replaces the inline try/except wraps in this PR. Trivial 3-way merge regardless of merge order; the inline wraps in this PR get replaced by the canonical helper call when the two land together.

Test Plan

• New: tests/integration/api/messages/test_messages_authz_api.py — 4/4 passing (authorized GET → 200, denied GET → 404 via collapse, non-existent message → 404 with explicit assertion that no authz call fires, authorized list happy-path).
• Regression: tests/integration/api/messages/test_messages_api.py — 19/19 passing.
• Regression: tests/integration/api/events/ — 3/3 passing (exercises the changed _get_parent_task_id signature on the event path).
• Regression: tests/integration/api/states/ — 7/7 passing (same, for state).
• Regression: tests/integration/api/agents/test_agents_auth_api.py — 3/3 passing (exercises the changed _ensure_authorized_id/_ensure_authorized_query factories on a direct-resource path).
• Ruff lint + format clean across all changed files.

Greptile Summary

Guards GET /messages/{message_id} with a task.read Spark AuthZ check by resolving the message to its parent task_id and collapsing AuthorizationError to 404, so callers cannot distinguish between "doesn't exist" and "exists but forbidden". The PR also corrects a dead-code None return branch on the get_message use case and adds full integration-test coverage for the auth matrix.

• message is added to TaskChildResourceType and wired through _get_parent_task_id, DAuthorizedId, and DAuthorizedQuery, which now each accept DTaskMessageRepository as an additional FastAPI dependency.
• The 404-collapse for AuthorizationError is applied to both path-id and query-id surfaces and incidentally hardens the event/state child routes as well.
• A new integration test file exercises the authorized, denied, nonexistent, and list happy-paths with verified authz payload assertions.

Confidence Score: 4/5

Safe to merge. The auth enforcement and 404-collapse logic are correct and well-tested; the only concern is an extra MongoDB round-trip per authorized GET request.

The core auth change is correct: the message is resolved to its parent task before the authz check, and denial collapses to 404. Tests cover all four relevant paths. The one observation is that the message document is fetched twice on the happy path — once in the auth dependency to extract task_id, and once more in the route handler — but this is an efficiency concern rather than a correctness issue.

No files require special attention; authorization_shortcuts.py carries the double-fetch pattern worth revisiting in a follow-up.

Important Files Changed

Filename	Overview
agentex/src/utils/authorization_shortcuts.py	Adds message_repository to _get_parent_task_id, _ensure_authorized_id, and _ensure_authorized_query; wraps the task-child authz check in try/except to collapse AuthorizationError → 404 on both path-id and query-id surfaces. Double-fetch of the message entity on the GET /messages/{id} path is a minor inefficiency.
agentex/src/api/routes/messages.py	Adds DAuthorizedId(TaskChildResourceType.message, read, param_name="message_id") to get_message; removes the dead
agentex/src/api/schemas/authorization_types.py	Adds message = "message" to TaskChildResourceType. Minimal, correct change.
agentex/src/domain/use_cases/messages_use_case.py	Corrects the get_message return annotation from TaskMessageEntity
agentex/tests/integration/api/messages/test_messages_authz_api.py	New integration test covering authorized GET→200, unauthorized GET→404 (collapse), non-existent message→404 (no authz call), and list happy-path. Fixtures and mock factory are well-structured; assertions verify both status codes and the exact authz payload.

Sequence Diagram

sequenceDiagram
    participant Client
    participant FastAPI as GET /messages/{id}
    participant AuthDep as DAuthorizedId dep
    participant MsgRepo as TaskMessageRepository
    participant AuthSvc as AuthorizationService
    participant UseCase as MessagesUseCase

    Client->>FastAPI: "GET /messages/{message_id}"
    FastAPI->>AuthDep: resolve dependency
    AuthDep->>MsgRepo: "get(id=message_id)"
    alt message not found
        MsgRepo-->>AuthDep: raises ItemDoesNotExist
        AuthDep-->>Client: 404
    else message found
        MsgRepo-->>AuthDep: TaskMessageEntity (task_id extracted)
        AuthDep->>AuthSvc: check(task, task_id, read)
        alt AuthorizationError
            AuthSvc-->>AuthDep: raises AuthorizationError
            AuthDep-->>Client: 404 (collapsed)
        else authorized
            AuthSvc-->>AuthDep: OK
            AuthDep-->>FastAPI: message_id string
            FastAPI->>UseCase: get_message(message_id)
            UseCase->>MsgRepo: "get(id=message_id) [second fetch]"
            MsgRepo-->>UseCase: TaskMessageEntity
            UseCase-->>FastAPI: TaskMessageEntity
            FastAPI-->>Client: 200 TaskMessage
        end
    end

Loading

Comments Outside Diff (1)

1. agentex/src/utils/authorization_shortcuts.py, line 48-81 (link)

   Double DB fetch for message on every GET /messages/{message_id} request

   _get_parent_task_id fetches the message entity from MongoDB to extract task_id, but the route handler then calls message_use_case.get_message a second time to retrieve and return the same document. For the common authorized path this means two round-trips to the messages collection per request. The event and state child types have the same pattern, but messages are likely to be a much higher-traffic endpoint. A future improvement would be to thread the already-fetched entity through the dependency (or cache it per-request) so the handler can skip the re-fetch.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: agentex/src/utils/authorization_shortcuts.py
   Line: 48-81
   
   Comment:
   **Double DB fetch for message on every `GET /messages/{message_id}` request**
   
   `_get_parent_task_id` fetches the message entity from MongoDB to extract `task_id`, but the route handler then calls `message_use_case.get_message` a second time to retrieve and return the same document. For the common authorized path this means two round-trips to the `messages` collection per request. The event and state child types have the same pattern, but messages are likely to be a much higher-traffic endpoint. A future improvement would be to thread the already-fetched entity through the dependency (or cache it per-request) so the handler can skip the re-fetch.
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

   

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:48-81
**Double DB fetch for message on every `GET /messages/{message_id}` request**

`_get_parent_task_id` fetches the message entity from MongoDB to extract `task_id`, but the route handler then calls `message_use_case.get_message` a second time to retrieve and return the same document. For the common authorized path this means two round-trips to the `messages` collection per request. The event and state child types have the same pattern, but messages are likely to be a much higher-traffic endpoint. A future improvement would be to thread the already-fetched entity through the dependency (or cache it per-request) so the handler can skip the re-fetch.

_{Reviews (1): Last reviewed commit: "refactor: rename task_message_repository..." | Re-trigger Greptile}