From e89edb39b17e3f623fbbcdc58580982596d906db Mon Sep 17 00:00:00 2001 From: Ryker_Feng <90562015+18062706139fcz@users.noreply.github.com> Date: Sat, 18 Jul 2026 22:12:57 +0800 Subject: [PATCH] fix(gateway): reject non-positive read limits (#4284) --- backend/app/gateway/routers/runs.py | 4 +- backend/app/gateway/routers/thread_runs.py | 14 +-- backend/tests/test_runs_api_endpoints.py | 15 ++++ .../tests/test_thread_run_query_validation.py | 87 +++++++++++++++++++ 4 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 backend/tests/test_thread_run_query_validation.py diff --git a/backend/app/gateway/routers/runs.py b/backend/app/gateway/routers/runs.py index 91ac155f4..2f09ac8ac 100644 --- a/backend/app/gateway/routers/runs.py +++ b/backend/app/gateway/routers/runs.py @@ -109,8 +109,8 @@ async def run_messages( run_id: str, request: Request, limit: int = Query(default=50, le=200, ge=1), - before_seq: int | None = Query(default=None), - after_seq: int | None = Query(default=None), + before_seq: int | None = Query(default=None, ge=1), + after_seq: int | None = Query(default=None, ge=1), ) -> dict: """Return paginated messages for a run (cursor-based). diff --git a/backend/app/gateway/routers/thread_runs.py b/backend/app/gateway/routers/thread_runs.py index a4de3b8ab..e0a8c1096 100644 --- a/backend/app/gateway/routers/thread_runs.py +++ b/backend/app/gateway/routers/thread_runs.py @@ -708,9 +708,9 @@ async def stream_existing_run( async def list_thread_messages( thread_id: str, request: Request, - limit: int = Query(default=50, le=200), - before_seq: int | None = Query(default=None), - after_seq: int | None = Query(default=None), + limit: int = Query(default=50, ge=1, le=200), + before_seq: int | None = Query(default=None, ge=1), + after_seq: int | None = Query(default=None, ge=1), ) -> list[dict]: """Return displayable messages for a thread (across all runs), with feedback attached.""" event_store = get_run_event_store(request) @@ -910,8 +910,8 @@ async def list_run_messages( run_id: str, request: Request, limit: int = Query(default=50, le=200, ge=1), - before_seq: int | None = Query(default=None), - after_seq: int | None = Query(default=None), + before_seq: int | None = Query(default=None, ge=1), + after_seq: int | None = Query(default=None, ge=1), ) -> dict: """Return paginated messages for a specific run. @@ -954,8 +954,8 @@ async def list_run_events( request: Request, event_types: str | None = Query(default=None), task_id: str | None = Query(default=None), - limit: int = Query(default=500, le=2000), - after_seq: int | None = Query(default=None), + limit: int = Query(default=500, ge=1, le=2000), + after_seq: int | None = Query(default=None, ge=1), ) -> list[dict]: """Return the full event stream for a run (debug/audit). diff --git a/backend/tests/test_runs_api_endpoints.py b/backend/tests/test_runs_api_endpoints.py index dfa5869c8..fbb7bdce3 100644 --- a/backend/tests/test_runs_api_endpoints.py +++ b/backend/tests/test_runs_api_endpoints.py @@ -4,6 +4,7 @@ from __future__ import annotations from unittest.mock import AsyncMock, MagicMock +import pytest from _router_auth_helpers import make_authed_test_app from _run_message_pagination_helpers import assert_run_message_page from fastapi.testclient import TestClient @@ -208,6 +209,20 @@ def test_run_messages_passes_before_seq_to_event_store(): ) +@pytest.mark.parametrize("cursor", ["before_seq", "after_seq"]) +@pytest.mark.parametrize("value", [0, -1]) +def test_run_messages_rejects_non_positive_seq_cursors(cursor: str, value: int): + run_record = {"run_id": "run-6", "thread_id": "thread-6"} + app = _make_app( + run_store=_make_run_store(run_record), + event_store=_make_event_store([]), + ) + with TestClient(app) as client: + response = client.get("/api/runs/run-6/messages", params={cursor: value}) + + assert response.status_code == 422 + + def test_run_messages_empty_data(): """Returns empty data list when no messages exist.""" run_record = {"run_id": "run-6", "thread_id": "thread-6"} diff --git a/backend/tests/test_thread_run_query_validation.py b/backend/tests/test_thread_run_query_validation.py new file mode 100644 index 000000000..0f4596610 --- /dev/null +++ b/backend/tests/test_thread_run_query_validation.py @@ -0,0 +1,87 @@ +"""Query validation for thread message and run event read endpoints.""" + +from unittest.mock import AsyncMock, MagicMock + +import pytest +from _router_auth_helpers import make_authed_test_app +from fastapi.testclient import TestClient + +from app.gateway.routers import thread_runs + + +def _make_app(): + app = make_authed_test_app() + app.include_router(thread_runs.router) + + event_store = MagicMock() + event_store.list_messages = AsyncMock(return_value=[]) + event_store.list_messages_by_run = AsyncMock(return_value=[]) + event_store.list_events = AsyncMock(return_value=[]) + app.state.run_event_store = event_store + + run_manager = MagicMock() + run_manager.list_by_thread = AsyncMock(return_value=[]) + app.state.run_manager = run_manager + return app + + +@pytest.mark.parametrize( + ("path", "limit"), + [ + ("/api/threads/thread-1/messages", 0), + ("/api/threads/thread-1/messages", -1), + ("/api/threads/thread-1/runs/run-1/events", 0), + ("/api/threads/thread-1/runs/run-1/events", -1), + ], +) +def test_read_endpoints_reject_non_positive_limits(path: str, limit: int): + with TestClient(_make_app()) as client: + response = client.get(path, params={"limit": limit}) + + assert response.status_code == 422 + + +@pytest.mark.parametrize( + ("path", "cursor"), + [ + ("/api/threads/thread-1/messages", "before_seq"), + ("/api/threads/thread-1/messages", "after_seq"), + ("/api/threads/thread-1/runs/run-1/messages", "before_seq"), + ("/api/threads/thread-1/runs/run-1/messages", "after_seq"), + ("/api/threads/thread-1/runs/run-1/events", "after_seq"), + ], +) +@pytest.mark.parametrize("value", [0, -1]) +def test_read_endpoints_reject_non_positive_seq_cursors(path: str, cursor: str, value: int): + with TestClient(_make_app()) as client: + response = client.get(path, params={cursor: value}) + + assert response.status_code == 422 + + +def test_read_endpoints_accept_positive_limits_and_hit_store(): + app = _make_app() + with TestClient(app) as client: + thread_messages = client.get("/api/threads/thread-1/messages", params={"limit": 1}) + run_messages = client.get("/api/threads/thread-1/runs/run-1/messages", params={"limit": 1}) + run_events = client.get("/api/threads/thread-1/runs/run-1/events", params={"limit": 1}) + + assert thread_messages.status_code == 200 + assert run_messages.status_code == 200 + assert run_events.status_code == 200 + app.state.run_event_store.list_messages.assert_awaited_once_with("thread-1", limit=1, before_seq=None, after_seq=None) + app.state.run_event_store.list_messages_by_run.assert_awaited_once_with( + "thread-1", + "run-1", + limit=2, + before_seq=None, + after_seq=None, + ) + app.state.run_event_store.list_events.assert_awaited_once_with( + "thread-1", + "run-1", + event_types=None, + task_id=None, + limit=1, + after_seq=None, + )