fix(api): validate annotation list pagination query (#36807)

Co-authored-by: root <kinsonnee@gmail.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

api/controllers/service_api/app/annotation.py

@@ -6,7 +6,7 @@ from flask_restx import Resource
 from flask_restx.api import HTTPStatus
 from pydantic import BaseModel, Field, TypeAdapter
 
-from controllers.common.schema import register_schema_models
+from controllers.common.schema import query_params_from_model, register_schema_models
 from controllers.console.wraps import edit_permission_required
 from controllers.service_api import service_api_ns
 from controllers.service_api.wraps import validate_app_token
@@ -32,8 +32,19 @@ class AnnotationReplyActionPayload(BaseModel):
     embedding_model_name: str = Field(description="Embedding model name")
 
 
+class AnnotationListQuery(BaseModel):
+    page: int = Field(default=1, ge=1, description="Page number")
+    limit: int = Field(default=20, ge=1, description="Number of annotations per page")
+    keyword: str = Field(default="", description="Keyword to search annotations")
+
+
 register_schema_models(
-    service_api_ns, AnnotationCreatePayload, AnnotationReplyActionPayload, Annotation, AnnotationList
+    service_api_ns,
+    AnnotationCreatePayload,
+    AnnotationReplyActionPayload,
+    AnnotationListQuery,
+    Annotation,
+    AnnotationList,
 )
 
 
@@ -100,6 +111,7 @@ class AnnotationReplyActionStatusApi(Resource):
 class AnnotationListApi(Resource):
     @service_api_ns.doc("list_annotations")
     @service_api_ns.doc(description="List annotations for the application")
+    @service_api_ns.doc(params=query_params_from_model(AnnotationListQuery))
     @service_api_ns.doc(
         responses={
             200: "Annotations retrieved successfully",
@@ -114,18 +126,18 @@ class AnnotationListApi(Resource):
     @validate_app_token
     def get(self, app_model: App):
         """List annotations for the application."""
-        page = request.args.get("page", default=1, type=int)
-        limit = request.args.get("limit", default=20, type=int)
-        keyword = request.args.get("keyword", default="", type=str)
+        query = AnnotationListQuery.model_validate(request.args.to_dict(flat=True))
 
-        annotation_list, total = AppAnnotationService.get_annotation_list_by_app_id(app_model.id, page, limit, keyword)
+        annotation_list, total = AppAnnotationService.get_annotation_list_by_app_id(
+            app_model.id, query.page, query.limit, query.keyword
+        )
         annotation_models = TypeAdapter(list[Annotation]).validate_python(annotation_list, from_attributes=True)
         response = AnnotationList(
             data=annotation_models,
-            has_more=len(annotation_list) == limit,
-            limit=limit,
+            has_more=len(annotation_list) == query.limit,
+            limit=query.limit,
             total=total,
-            page=page,
+            page=query.page,
         )
         return response.model_dump(mode="json")
 

api/openapi/markdown/service-swagger.md

@@ -112,6 +112,14 @@ List annotations for the application
 
 List annotations for the application
 
+##### Parameters
+
+| Name | Located in | Description | Required | Schema |
+| ---- | ---------- | ----------- | -------- | ------ |
+| keyword | query | Keyword to search annotations | No | string |
+| limit | query | Number of annotations per page | No | integer |
+| page | query | Page number | No | integer |
+
 ##### Responses
 
 | Code | Description | Schema |
@@ -2169,6 +2177,14 @@ Returns a list of available models for the specified model type.
 | page | integer |  | Yes |
 | total | integer |  | Yes |
 
+#### AnnotationListQuery
+
+| Name | Type | Description | Required |
+| ---- | ---- | ----------- | -------- |
+| keyword | string | Keyword to search annotations | No |
+| limit | integer | Number of annotations per page | No |
+| page | integer | Page number | No |
+
 #### AnnotationReplyActionPayload
 
 | Name | Type | Description | Required |

api/tests/unit_tests/controllers/service_api/app/test_annotation.py

@@ -19,10 +19,12 @@ from unittest.mock import Mock
 import pytest
 from flask import Flask
 from flask_restx.api import HTTPStatus
+from pydantic import ValidationError
 
 from controllers.service_api.app.annotation import (
     AnnotationCreatePayload,
     AnnotationListApi,
+    AnnotationListQuery,
     AnnotationReplyActionApi,
     AnnotationReplyActionPayload,
     AnnotationReplyActionStatusApi,
@@ -106,6 +108,28 @@ class TestAnnotationReplyActionPayload:
         assert payload.score_threshold == 0.0
 
 
+class TestAnnotationListQuery:
+    def test_defaults(self) -> None:
+        query = AnnotationListQuery.model_validate({})
+
+        assert query.page == 1
+        assert query.limit == 20
+        assert query.keyword == ""
+
+    def test_valid_numeric_strings(self) -> None:
+        query = AnnotationListQuery.model_validate({"page": "2", "limit": "5", "keyword": "refund"})
+
+        assert query.page == 2
+        assert query.limit == 5
+        assert query.keyword == "refund"
+
+    @pytest.mark.parametrize("field", ["page", "limit"])
+    @pytest.mark.parametrize("value", ["abc", "1.5", "1e2", "", "0", "-1"])
+    def test_invalid_explicit_pagination_value(self, field: str, value: str) -> None:
+        with pytest.raises(ValidationError):
+            AnnotationListQuery.model_validate({field: value})
+
+
 # ---------------------------------------------------------------------------
 # Model and Error Pattern Tests
 # ---------------------------------------------------------------------------
@@ -232,22 +256,55 @@ class TestAnnotationReplyActionStatusApi:
 
 
 class TestAnnotationListApi:
-    def test_get(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None:
+    def test_get_uses_defaults(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None:
         annotation = SimpleNamespace(id="a1", question="q", content="a", created_at=0)
-        monkeypatch.setattr(
-            AppAnnotationService,
-            "get_annotation_list_by_app_id",
-            lambda *_args, **_kwargs: ([annotation], 1),
-        )
+        get_mock = Mock(return_value=([annotation], 1))
+        monkeypatch.setattr(AppAnnotationService, "get_annotation_list_by_app_id", get_mock)
 
         api = AnnotationListApi()
         handler = _unwrap(api.get)
         app_model = SimpleNamespace(id="app")
 
-        with app.test_request_context("/apps/annotations?page=1&limit=1", method="GET"):
+        with app.test_request_context("/apps/annotations", method="GET"):
+            response = handler(api, app_model=app_model)
+
+        assert response["page"] == 1
+        assert response["limit"] == 20
+        get_mock.assert_called_once_with("app", 1, 20, "")
+
+    def test_get_accepts_valid_numeric_strings(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None:
+        annotation = SimpleNamespace(id="a1", question="q", content="a", created_at=0)
+        get_mock = Mock(return_value=([annotation], 1))
+        monkeypatch.setattr(AppAnnotationService, "get_annotation_list_by_app_id", get_mock)
+
+        api = AnnotationListApi()
+        handler = _unwrap(api.get)
+        app_model = SimpleNamespace(id="app")
+
+        with app.test_request_context("/apps/annotations?page=2&limit=5&keyword=refund", method="GET"):
             response = handler(api, app_model=app_model)
 
         assert response["total"] == 1
+        assert response["page"] == 2
+        assert response["limit"] == 5
+        get_mock.assert_called_once_with("app", 2, 5, "refund")
+
+    @pytest.mark.parametrize("query_string", ["page=abc&limit=5", "page=1&limit=abc", "page=&limit=5", "limit=0"])
+    def test_get_rejects_invalid_explicit_pagination_value(
+        self, app: Flask, monkeypatch: pytest.MonkeyPatch, query_string: str
+    ) -> None:
+        get_mock = Mock(return_value=([], 0))
+        monkeypatch.setattr(AppAnnotationService, "get_annotation_list_by_app_id", get_mock)
+
+        api = AnnotationListApi()
+        handler = _unwrap(api.get)
+        app_model = SimpleNamespace(id="app")
+
+        with app.test_request_context(f"/apps/annotations?{query_string}", method="GET"):
+            with pytest.raises(ValidationError):
+                handler(api, app_model=app_model)
+
+        get_mock.assert_not_called()
 
     def test_create(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None:
         annotation = SimpleNamespace(id="a1", question="q", content="a", created_at=0)

packages/contracts/generated/api/service/orpc.gen.ts

@@ -24,6 +24,7 @@ import {
   zGetAppFeedbacksResponse,
   zGetAppsAnnotationReplyByActionStatusByJobIdPath,
   zGetAppsAnnotationReplyByActionStatusByJobIdResponse,
+  zGetAppsAnnotationsQuery,
   zGetAppsAnnotationsResponse,
   zGetConversationsByCIdVariablesPath,
   zGetConversationsByCIdVariablesQuery,
@@ -379,6 +380,7 @@ export const get4 = oc
     summary: 'List annotations for the application',
     tags: ['service_api'],
   })
+  .input(z.object({ query: zGetAppsAnnotationsQuery.optional() }))
   .output(zGetAppsAnnotationsResponse)
 
 /**

packages/contracts/generated/api/service/types.gen.ts

@@ -25,6 +25,12 @@ export type AnnotationList = {
   total: number
 }
 
+export type AnnotationListQuery = {
+  keyword?: string
+  limit?: number
+  page?: number
+}
+
 export type AnnotationReplyActionPayload = {
   embedding_model_name: string
   embedding_provider_name: string
@@ -969,7 +975,11 @@ export type GetAppsAnnotationReplyByActionStatusByJobIdResponse
 export type GetAppsAnnotationsData = {
   body?: never
   path?: never
-  query?: never
+  query?: {
+    keyword?: string
+    limit?: number
+    page?: number
+  }
   url: '/apps/annotations'
 }
 

packages/contracts/generated/api/service/zod.gen.ts

@@ -32,6 +32,15 @@ export const zAnnotationList = z.object({
   total: z.int(),
 })
 
+/**
+ * AnnotationListQuery
+ */
+export const zAnnotationListQuery = z.object({
+  keyword: z.string().optional().default(''),
+  limit: z.int().gte(1).optional().default(20),
+  page: z.int().gte(1).optional().default(1),
+})
+
 /**
  * AnnotationReplyActionPayload
  */
@@ -1216,6 +1225,12 @@ export const zGetAppsAnnotationReplyByActionStatusByJobIdResponse = z.record(
   z.unknown(),
 )
 
+export const zGetAppsAnnotationsQuery = z.object({
+  keyword: z.string().optional().default(''),
+  limit: z.int().gte(1).optional().default(20),
+  page: z.int().gte(1).optional().default(1),
+})
+
 /**
  * Annotations retrieved successfully
  */