From d2d338c13ac5f1c1b89b4a3b4338cd14fbf86efc Mon Sep 17 00:00:00 2001 From: Makoto Kurihara Date: Sat, 12 Sep 2026 21:47:30 +0900 Subject: [PATCH 1/2] fix(agent-server): enforce the paginated search limit instead of asserting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four paginated search endpoints declared their page-size bound as `Query(gt=0, lte=100)`. `lte` is not a FastAPI/Pydantic constraint keyword (`le` is) and an unknown key does not raise — it rides along into the JSON schema and is never enforced. `limit=101` therefore passed validation and reached a bare `assert limit <= 100` in the handler, which surfaced as an unhandled AssertionError: HTTP 500 for what is a client error. Two consequences: the cap never appeared in the OpenAPI document, so a generated client could not see it, and exceeding it looked like a server fault rather than a 422. Use `le=100` and drop the now-redundant asserts. The bound is unchanged, so requests that worked before still work; oversized ones now get 422 instead of 500. An existing test asserted the AssertionError, so it is updated to expect the 422. Affected: conversations/search, conversations/{id}/events/search, bash/bash_events/search, file/search_subdirs. Co-Authored-By: Claude Opus 5 --- .../openhands/agent_server/bash_router.py | 4 +- .../agent_server/conversation_router.py | 4 +- .../openhands/agent_server/event_router.py | 4 +- .../openhands/agent_server/file_router.py | 4 +- .../agent_server/test_conversation_router.py | 7 +- .../test_search_limit_validation.py | 85 +++++++++++++++++++ 6 files changed, 92 insertions(+), 16 deletions(-) create mode 100644 tests/agent_server/test_search_limit_validation.py diff --git a/openhands-agent-server/openhands/agent_server/bash_router.py b/openhands-agent-server/openhands/agent_server/bash_router.py index b6188e6d163..552c101ea79 100644 --- a/openhands-agent-server/openhands/agent_server/bash_router.py +++ b/openhands-agent-server/openhands/agent_server/bash_router.py @@ -52,13 +52,11 @@ async def search_bash_events( ] = None, limit: Annotated[ int, - Query(title="The max number of results in the page", gt=0, lte=100), + Query(title="The max number of results in the page", gt=0, le=100), ] = 100, bash_event_service: BashEventService = Depends(get_bash_event_service), ) -> BashEventPage: """Search / List bash event events""" - assert limit > 0 - assert limit <= 100 return await bash_event_service.search_bash_events( kind__eq=kind__eq, diff --git a/openhands-agent-server/openhands/agent_server/conversation_router.py b/openhands-agent-server/openhands/agent_server/conversation_router.py index 6e252f667c1..32aea829498 100644 --- a/openhands-agent-server/openhands/agent_server/conversation_router.py +++ b/openhands-agent-server/openhands/agent_server/conversation_router.py @@ -94,7 +94,7 @@ async def search_conversations( ] = None, limit: Annotated[ int, - Query(title="The max number of results in the page", gt=0, lte=100), + Query(title="The max number of results in the page", gt=0, le=100), ] = 100, status: Annotated[ ConversationExecutionStatus | None, @@ -108,8 +108,6 @@ async def search_conversations( conversation_service: ConversationService = Depends(get_conversation_service), ) -> ConversationPage: """Search / List conversations""" - assert limit > 0 - assert limit <= 100 page = await conversation_service.search_conversations( page_id, limit, status, sort_order ) diff --git a/openhands-agent-server/openhands/agent_server/event_router.py b/openhands-agent-server/openhands/agent_server/event_router.py index 7358cdadf92..c40e7f4f3e1 100644 --- a/openhands-agent-server/openhands/agent_server/event_router.py +++ b/openhands-agent-server/openhands/agent_server/event_router.py @@ -70,7 +70,7 @@ async def search_conversation_events( ] = None, limit: Annotated[ int, - Query(title="The max number of results in the page", gt=0, lte=100), + Query(title="The max number of results in the page", gt=0, le=100), ] = 100, kind: Annotated[ str | None, @@ -101,8 +101,6 @@ async def search_conversation_events( event_service: EventService = Depends(get_event_service), ) -> JSONResponse: """Search / List local events""" - assert limit > 0 - assert limit <= 100 # Normalize timezone-aware datetimes to server timezone normalized_gte = ( diff --git a/openhands-agent-server/openhands/agent_server/file_router.py b/openhands-agent-server/openhands/agent_server/file_router.py index 8c3f63d47e2..6c3af23fe43 100644 --- a/openhands-agent-server/openhands/agent_server/file_router.py +++ b/openhands-agent-server/openhands/agent_server/file_router.py @@ -792,7 +792,7 @@ async def search_subdirs( ] = None, limit: Annotated[ int, - Query(title="The max number of results in the page", gt=0, lte=100), + Query(title="The max number of results in the page", gt=0, le=100), ] = 100, include_hidden: Annotated[ bool, @@ -810,8 +810,6 @@ async def search_subdirs( the ``next_page_id`` returned by the previous page (the lowercase name of the first item to include on the next page). """ - assert limit > 0 - assert limit <= 100 target = Path(path) if not target.is_absolute(): diff --git a/tests/agent_server/test_conversation_router.py b/tests/agent_server/test_conversation_router.py index 22158ef5a72..f0d3c8c0f90 100644 --- a/tests/agent_server/test_conversation_router.py +++ b/tests/agent_server/test_conversation_router.py @@ -204,10 +204,9 @@ def test_search_conversations_limit_validation(client, mock_conversation_service response = client.get("/api/conversations/search", params={"limit": 0}) assert response.status_code == 422 - # Test limit too high - endpoint has FastAPI validation (lte=100) and assertion - # The assertion in the endpoint will cause an AssertionError to be raised - with pytest.raises(AssertionError): - response = client.get("/api/conversations/search", params={"limit": 101}) + # Test limit too high - rejected by FastAPI validation (le=100) + response = client.get("/api/conversations/search", params={"limit": 101}) + assert response.status_code == 422 # Test valid limit mock_conversation_service.search_conversations.return_value = ConversationPage( diff --git a/tests/agent_server/test_search_limit_validation.py b/tests/agent_server/test_search_limit_validation.py new file mode 100644 index 00000000000..4b724fcb597 --- /dev/null +++ b/tests/agent_server/test_search_limit_validation.py @@ -0,0 +1,85 @@ +"""The ``limit`` bound on every paginated search endpoint must be enforced. + +These endpoints used to declare ``Query(gt=0, lte=100)``. ``lte`` is not a +FastAPI/Pydantic constraint keyword (``le`` is), so the bound was silently +dropped: it never reached the OpenAPI schema and was never validated. An +oversized ``limit`` then hit a bare ``assert`` in the handler and surfaced as an +unhandled ``AssertionError`` — HTTP 500 for what is a client error. +""" + +from unittest.mock import AsyncMock +from uuid import uuid4 + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from openhands.agent_server.bash_router import bash_router +from openhands.agent_server.conversation_router import conversation_router +from openhands.agent_server.dependencies import get_event_service +from openhands.agent_server.event_router import event_router +from openhands.agent_server.event_service import EventService +from openhands.agent_server.file_router import file_router + + +SEARCH_ROUTES = [ + (conversation_router, "/api/conversations/search"), + (event_router, "/api/conversations/{conversation_id}/events/search"), + (bash_router, "/api/bash/bash_events/search"), + (file_router, "/api/file/search_subdirs"), +] + + +def _limit_schema(router, path: str) -> dict: + app = FastAPI() + app.include_router(router, prefix="/api") + parameters = app.openapi()["paths"][path]["get"]["parameters"] + limit = next(p for p in parameters if p["name"] == "limit") + return limit["schema"] + + +@pytest.mark.parametrize( + "router, path", SEARCH_ROUTES, ids=[path for _, path in SEARCH_ROUTES] +) +def test_search_limit_bounds_are_published_in_the_schema(router, path): + """The bound has to be in the schema, or generated clients cannot see it.""" + schema = _limit_schema(router, path) + + assert schema["maximum"] == 100 + assert schema["exclusiveMinimum"] == 0 + # A misspelled constraint rides along into the schema instead of raising. + assert "lte" not in schema + + +@pytest.mark.parametrize("limit", [0, 101]) +def test_search_events_rejects_out_of_range_limit(limit): + """An out-of-range ``limit`` is a client error, not a server fault.""" + app = FastAPI() + app.include_router(event_router, prefix="/api") + service = AsyncMock(spec=EventService) + service.search_events = AsyncMock(return_value={"items": [], "next_page_id": None}) + app.dependency_overrides[get_event_service] = lambda: service + + with TestClient(app) as client: + response = client.get( + f"/api/conversations/{uuid4()}/events/search", params={"limit": limit} + ) + + assert response.status_code == 422 + service.search_events.assert_not_called() + + +def test_search_events_accepts_the_maximum_limit(): + app = FastAPI() + app.include_router(event_router, prefix="/api") + service = AsyncMock(spec=EventService) + service.search_events = AsyncMock(return_value={"items": [], "next_page_id": None}) + app.dependency_overrides[get_event_service] = lambda: service + + with TestClient(app) as client: + response = client.get( + f"/api/conversations/{uuid4()}/events/search", params={"limit": 100} + ) + + assert response.status_code == 200 + service.search_events.assert_called_once() From 27865fc2581a16a8c6aa49c6a22006f9fbf2b645 Mon Sep 17 00:00:00 2001 From: enyst Date: Mon, 14 Sep 2026 20:52:56 +0000 Subject: [PATCH 2/2] test(agent-server): import file_discovery_router for search_subdirs limit test The search_subdirs route lives on file_discovery_router, not file_router. The test previously imported file_router, so /api/file/search_subdirs was absent from its OpenAPI schema and the schema assertion raised KeyError. Co-authored-by: openhands --- tests/agent_server/test_search_limit_validation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/agent_server/test_search_limit_validation.py b/tests/agent_server/test_search_limit_validation.py index 4b724fcb597..a857341c6d5 100644 --- a/tests/agent_server/test_search_limit_validation.py +++ b/tests/agent_server/test_search_limit_validation.py @@ -19,14 +19,14 @@ from openhands.agent_server.dependencies import get_event_service from openhands.agent_server.event_router import event_router from openhands.agent_server.event_service import EventService -from openhands.agent_server.file_router import file_router +from openhands.agent_server.file_router import file_discovery_router SEARCH_ROUTES = [ (conversation_router, "/api/conversations/search"), (event_router, "/api/conversations/{conversation_id}/events/search"), (bash_router, "/api/bash/bash_events/search"), - (file_router, "/api/file/search_subdirs"), + (file_discovery_router, "/api/file/search_subdirs"), ]