feat(api,cli): strict UUID validation for app-id and workspace-id (#37212)

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

api/controllers/openapi/_models.py

@@ -6,7 +6,7 @@ from typing import Any, Literal
 
 from pydantic import BaseModel, ConfigDict, Field, field_validator
 
-from libs.helper import EmailStr, UUIDStrOrEmpty, uuid_value
+from libs.helper import EmailStr, UUIDStr, UUIDStrOrEmpty, uuid_value
 from models.model import AppMode
 
 # Server-side cap on `limit` query param for /openapi/v1/* list endpoints.
@@ -262,22 +262,6 @@ class AppDescribeQuery(BaseModel):
     model_config = ConfigDict(extra="forbid")
 
     fields: set[str] | None = Field(default=None, json_schema_extra=_csv_string_query_schema)
-    workspace_id: str | None = None
-
-    @field_validator("workspace_id", mode="before")
-    @classmethod
-    def _validate_workspace_id(cls, v: object) -> str | None:
-        if v is None or v == "":
-            return None
-        if not isinstance(v, str):
-            raise ValueError("workspace_id must be a string")
-        try:
-            import uuid as _uuid
-
-            _uuid.UUID(v)
-        except ValueError:
-            raise ValueError("workspace_id must be a valid UUID")
-        return v
 
     @field_validator("fields", mode="before")
     @classmethod
@@ -297,7 +281,7 @@ class AppDescribeQuery(BaseModel):
 class AppListQuery(BaseModel):
     """mode is a closed enum."""
 
-    workspace_id: str
+    workspace_id: UUIDStr
     page: int = Field(1, ge=1)
     limit: int = Field(20, ge=1, le=MAX_PAGE_LIMIT)
     mode: AppMode | None = None

api/controllers/openapi/apps.py

@@ -97,7 +97,7 @@ class AppDescribeApi(AppReadResource):
         except ValidationError as exc:
             raise UnprocessableEntity(exc.json())
 
-        app = self._load(app_id, workspace_id=query.workspace_id)
+        app = self._load(app_id)
 
         requested = query.fields
         want_info = requested is None or "info" in requested

api/controllers/openapi/auth/prepare.py

@@ -19,6 +19,10 @@ def load_app(data: AuthData) -> None:
     if data.app is not None:
         return
     app_id = data.path_params["app_id"]
+    try:
+        uuid.UUID(app_id)
+    except ValueError:
+        raise NotFound("app not found")
     app = AppService.get_app_by_id(db.session, app_id)
     if not app or app.status != "normal":
         raise NotFound("app not found")

api/libs/helper.py

@@ -273,6 +273,18 @@ def parse_uuid_str_or_none(value: str | None) -> str | None:
 UUIDStrOrEmpty = Annotated[str, AfterValidator(normalize_uuid)]
 
 
+def _strict_uuid(value: str | UUID) -> str:
+    if not value:
+        raise ValueError("must be a non-empty valid UUID")
+    try:
+        return uuid_value(value)
+    except ValueError as exc:
+        raise ValueError("must be a valid UUID") from exc
+
+
+UUIDStr = Annotated[str, AfterValidator(_strict_uuid)]
+
+
 def alphanumeric(value: str):
     # check if the value is alphanumeric and underlined
     if re.match(r"^[a-zA-Z0-9_]+$", value):

api/openapi/markdown/openapi-swagger.md

@@ -112,7 +112,6 @@ User-scoped operations
 | ---- | ---------- | ----------- | -------- | ------ |
 | app_id | path |  | Yes | string |
 | fields | query |  | No | string |
-| workspace_id | query |  | No | string |
 
 ##### Responses
 
@@ -454,7 +453,6 @@ Empty / omitted → all blocks. Unknown member → ValidationError → 422.
 | Name | Type | Description | Required |
 | ---- | ---- | ----------- | -------- |
 | fields | string |  | No |
-| workspace_id | string |  | No |
 
 #### AppDescribeResponse
 

api/tests/unit_tests/controllers/openapi/auth/test_prepare.py

@@ -32,18 +32,27 @@ def _make_auth_data(**kwargs) -> AuthData:
     return data
 
 
+_VALID_APP_UUID = "00000000-0000-0000-0000-000000000001"
+
+
 def test_load_app_writes_app_to_data():
     app = MagicMock()
     app.status = "normal"
     app.enable_api = True
-    data = _make_auth_data(path_params={"app_id": "abc"})
+    data = _make_auth_data(path_params={"app_id": _VALID_APP_UUID})
     with patch("controllers.openapi.auth.prepare.AppService.get_app_by_id", return_value=app):
         load_app(data)
     assert data.app is app
 
 
+def test_load_app_raises_not_found_for_non_uuid_app_id():
+    data = _make_auth_data(path_params={"app_id": "not-a-uuid"})
+    with pytest.raises(NotFound):
+        load_app(data)
+
+
 def test_load_app_raises_not_found_when_missing():
-    data = _make_auth_data(path_params={"app_id": "missing"})
+    data = _make_auth_data(path_params={"app_id": _VALID_APP_UUID})
     with patch("controllers.openapi.auth.prepare.AppService.get_app_by_id", return_value=None):
         with pytest.raises(NotFound):
             load_app(data)
@@ -52,7 +61,7 @@ def test_load_app_raises_not_found_when_missing():
 def test_load_app_raises_not_found_when_not_normal():
     app = MagicMock()
     app.status = "archived"
-    data = _make_auth_data(path_params={"app_id": "abc"})
+    data = _make_auth_data(path_params={"app_id": _VALID_APP_UUID})
     with patch("controllers.openapi.auth.prepare.AppService.get_app_by_id", return_value=app):
         with pytest.raises(NotFound):
             load_app(data)
@@ -62,7 +71,7 @@ def test_load_app_stashes_app_even_when_api_disabled():
     app = MagicMock()
     app.status = "normal"
     app.enable_api = False
-    data = _make_auth_data(path_params={"app_id": "abc"})
+    data = _make_auth_data(path_params={"app_id": _VALID_APP_UUID})
     with patch("controllers.openapi.auth.prepare.AppService.get_app_by_id", return_value=app):
         load_app(data)
     assert data.app is app

api/tests/unit_tests/controllers/openapi/test_app_list_query.py

@@ -18,8 +18,8 @@ from controllers.openapi.apps import AppListQuery
 
 
 def test_defaults():
-    q = AppListQuery.model_validate({"workspace_id": "ws-1"})
-    assert q.workspace_id == "ws-1"
+    q = AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001"})
+    assert q.workspace_id == "00000000-0000-0000-0000-000000000001"
     assert q.page == 1
     assert q.limit == 20
     assert q.mode is None
@@ -34,61 +34,63 @@ def test_workspace_id_required():
 
 def test_page_must_be_positive():
     with pytest.raises(ValidationError):
-        AppListQuery.model_validate({"workspace_id": "ws-1", "page": 0})
+        AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "page": 0})
     with pytest.raises(ValidationError):
-        AppListQuery.model_validate({"workspace_id": "ws-1", "page": -1})
+        AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "page": -1})
 
 
 def test_page_rejects_non_integer_string():
     with pytest.raises(ValidationError):
-        AppListQuery.model_validate({"workspace_id": "ws-1", "page": "abc"})
+        AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "page": "abc"})
 
 
 def test_limit_must_be_positive():
     with pytest.raises(ValidationError):
-        AppListQuery.model_validate({"workspace_id": "ws-1", "limit": 0})
+        AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "limit": 0})
     with pytest.raises(ValidationError):
-        AppListQuery.model_validate({"workspace_id": "ws-1", "limit": -1})
+        AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "limit": -1})
 
 
 def test_limit_caps_at_max_page_limit():
     # Boundary accepts.
-    q = AppListQuery.model_validate({"workspace_id": "ws-1", "limit": MAX_PAGE_LIMIT})
+    q = AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "limit": MAX_PAGE_LIMIT})
     assert q.limit == MAX_PAGE_LIMIT
 
     # Just over rejects.
     with pytest.raises(ValidationError):
-        AppListQuery.model_validate({"workspace_id": "ws-1", "limit": MAX_PAGE_LIMIT + 1})
+        AppListQuery.model_validate(
+            {"workspace_id": "00000000-0000-0000-0000-000000000001", "limit": MAX_PAGE_LIMIT + 1}
+        )
 
 
 def test_mode_whitelisted_against_app_mode():
     # Valid mode passes.
-    q = AppListQuery.model_validate({"workspace_id": "ws-1", "mode": "chat"})
+    q = AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "mode": "chat"})
     assert q.mode is not None
     assert q.mode.value == "chat"
 
     # Invalid mode rejects.
     with pytest.raises(ValidationError):
-        AppListQuery.model_validate({"workspace_id": "ws-1", "mode": "not-a-mode"})
+        AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "mode": "not-a-mode"})
 
 
 def test_name_length_capped():
-    AppListQuery.model_validate({"workspace_id": "ws-1", "name": "x" * 200})
+    AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "name": "x" * 200})
     with pytest.raises(ValidationError):
-        AppListQuery.model_validate({"workspace_id": "ws-1", "name": "x" * 201})
+        AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "name": "x" * 201})
 
 
 def test_tag_length_capped():
-    AppListQuery.model_validate({"workspace_id": "ws-1", "tag": "x" * 100})
+    AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "tag": "x" * 100})
     with pytest.raises(ValidationError):
-        AppListQuery.model_validate({"workspace_id": "ws-1", "tag": "x" * 101})
+        AppListQuery.model_validate({"workspace_id": "00000000-0000-0000-0000-000000000001", "tag": "x" * 101})
 
 
 def test_all_fields_accept_valid_values():
     """Pin the happy-path acceptance for every field in one place."""
     q = AppListQuery.model_validate(
         {
-            "workspace_id": "ws-1",
+            "workspace_id": "00000000-0000-0000-0000-000000000001",
             "page": 5,
             "limit": 50,
             "mode": "workflow",
@@ -96,7 +98,7 @@ def test_all_fields_accept_valid_values():
             "tag": "prod",
         }
     )
-    assert q.workspace_id == "ws-1"
+    assert q.workspace_id == "00000000-0000-0000-0000-000000000001"
     assert q.page == 5
     assert q.limit == 50
     assert q.mode is not None