fix: harden project writes and uploads

nocobase_crm_mcp/nocobase_http.py

@@ -2,6 +2,7 @@ from __future__ import annotations
 
 from pathlib import Path
 from typing import Any, Sequence
+from urllib.parse import quote
 
 import httpx
 
@@ -57,8 +58,9 @@ class NocoBaseHttpClient:
         self._post_attachment_ids(project_id, attachment_ids, "remove")
 
     def _post_attachment_ids(self, project_id: Any, attachment_ids: Sequence[Any], action: str) -> None:
+        project_id_segment = _project_id_segment(project_id)
         response = self._client.post(
-            f"/t_projects/{project_id}/f_projects_attachments:{action}",
+            f"/t_projects/{project_id_segment}/f_projects_attachments:{action}",
             json={"tk": list(attachment_ids)},
         )
         self._raise_for_error(response)
@@ -81,3 +83,11 @@ class NocoBaseHttpClient:
 
     def _redact(self, value: str) -> str:
         return value.replace(self.jwt, "<redacted>")
+
+
+def _project_id_segment(project_id: Any) -> str:
+    if isinstance(project_id, bool) or not isinstance(project_id, (str, int)):
+        raise ValueError("project_id must be a scalar identifier")
+    if isinstance(project_id, str) and not project_id.strip():
+        raise ValueError("project_id must not be blank")
+    return quote(str(project_id), safe="")

nocobase_crm_mcp/projects.py

@@ -97,6 +97,7 @@ class ProjectService:
         remove_attachment_ids: Sequence[Any] | None = None,
     ) -> dict[str, Any]:
         try:
+            _validate_identifier(project_id, "project_id")
             values = self._project_values(
                 name=name,
                 customer_id=customer_id,
@@ -153,6 +154,7 @@ class ProjectService:
             raise _InvalidProjectInput("name is required")
 
         if customer_id is not None:
+            _validate_identifier(customer_id, "customer_id")
             values[self.config.customer_field] = customer_id
         if inquiry_date is not None:
             if not isinstance(inquiry_date, str):
@@ -165,7 +167,9 @@ class ProjectService:
             except ValueError as error:
                 raise _InvalidProjectInput("inquiry_date must be a YYYY-MM-DD date") from error
         if progress is not None:
-            if not isinstance(progress, str) or progress not in self.config.progress_values:
+            if not isinstance(progress, str):
+                raise _InvalidProjectInput("progress must be a scalar value")
+            if progress not in self.config.progress_values:
                 raise _InvalidProgress
             values[self.config.progress_field] = self.config.progress_values[progress]
 
@@ -176,6 +180,8 @@ class ProjectService:
             (details, self.config.details_field),
         ):
             if value is not None:
+                if not isinstance(value, str):
+                    raise _InvalidProjectInput(f"{field} must be a scalar value")
                 values[field] = value
         return values
 
@@ -188,7 +194,11 @@ def _deduplicated_attachment_ids(attachment_ids: Sequence[Any] | None) -> list[A
 
     unique_ids: list[Any] = []
     for attachment_id in attachment_ids:
-        if attachment_id is None or isinstance(attachment_id, (Mapping, Sequence)):
+        try:
+            _validate_identifier(attachment_id, "attachment IDs")
+        except _InvalidProjectInput as error:
+            raise _InvalidProjectInput("attachment IDs must be scalar values") from error
+        if attachment_id is None:
             raise _InvalidProjectInput("attachment IDs must be scalar values")
         if attachment_id not in unique_ids:
             unique_ids.append(attachment_id)
@@ -206,6 +216,13 @@ def _project_id(response: Mapping[str, Any], id_field: str) -> Any:
     return data[id_field]
 
 
+def _validate_identifier(value: Any, name: str) -> None:
+    if isinstance(value, bool) or not isinstance(value, (str, int)):
+        raise _InvalidProjectInput(f"{name} must be a scalar identifier")
+    if isinstance(value, str) and not value.strip():
+        raise _InvalidProjectInput(f"{name} must not be blank")
+
+
 def _attachment_error(project_id: Any, error: Exception) -> dict[str, Any]:
     if isinstance(error, NocoBaseHttpError):
         if error.status_code == 404:

nocobase_crm_mcp/uploads.py

@@ -14,6 +14,11 @@ from .nocobase_http import NocoBaseHttpError
 
 
 _COPY_CHUNK_BYTES = 1024 * 1024
+_MULTIPART_OVERHEAD_BYTES = 64 * 1024
+
+
+class _UploadTooLarge(ValueError):
+    pass
 
 
 class UploadEndpoint:
@@ -38,17 +43,20 @@ class UploadEndpoint:
             return _error_response("invalid_upload", "Uploads require multipart form data", 400)
 
         try:
+            request = await _bounded_request(request, self._max_bytes + _MULTIPART_OVERHEAD_BYTES)
             form = await request.form()
+        except _UploadTooLarge:
+            return _error_response("upload_too_large", "Upload exceeds the configured size limit", 413)
         except Exception:
             return _error_response("invalid_upload", "Could not read upload form data", 400)
 
-        uploads = [(name, value) for name, value in form.multi_items() if isinstance(value, UploadFile)]
-        file_values = form.getlist("file")
-        if len(uploads) != 1 or len(file_values) != 1 or uploads[0][0] != "file":
+        items = list(form.multi_items())
+        uploads = [(name, value) for name, value in items if isinstance(value, UploadFile)]
+        if len(items) != 1 or len(uploads) != 1 or uploads[0][0] != "file":
             await _close_uploads(value for _, value in uploads)
             return _error_response("invalid_upload", "Provide exactly one file upload", 400)
 
-        upload = file_values[0]
+        upload = uploads[0][1]
         if not isinstance(upload, UploadFile):
             return _error_response("invalid_upload", "Provide exactly one file upload", 400)
 
@@ -94,6 +102,43 @@ def _is_multipart(request: Request) -> bool:
     return content_type.split(";", 1)[0].strip().lower() == "multipart/form-data"
 
 
+async def _bounded_request(request: Request, max_body_bytes: int) -> Request:
+    content_length = request.headers.get("content-length")
+    if content_length is not None:
+        try:
+            if int(content_length) > max_body_bytes:
+                raise _UploadTooLarge
+        except ValueError:
+            pass
+
+    received_bytes = 0
+    chunks: list[bytes] = []
+    while True:
+        message = await request.receive()
+        if message.get("type") == "http.request":
+            body = message.get("body", b"")
+            received_bytes += len(body)
+            if received_bytes > max_body_bytes:
+                raise _UploadTooLarge
+            chunks.append(body)
+            if not message.get("more_body", False):
+                break
+        elif message.get("type") == "http.disconnect":
+            break
+
+    body = b"".join(chunks)
+    replayed = False
+
+    async def receive() -> dict[str, Any]:
+        nonlocal replayed
+        if replayed:
+            return {"type": "http.disconnect"}
+        replayed = True
+        return {"type": "http.request", "body": body, "more_body": False}
+
+    return Request(request.scope, receive=receive)
+
+
 def _attachment_id(attachment: Any) -> int | None:
     if not isinstance(attachment, Mapping):
         return None

tests/test_nocobase_http.py

@@ -67,6 +67,31 @@ class NocoBaseHttpClientTests(unittest.TestCase):
         self.assertEqual(str(requests[1].url), "https://nb.example/api/t_projects/7/f_projects_attachments:remove")
         self.assertEqual(json.loads(requests[1].content), {"tk": [10]})
 
+    def test_attachment_association_encodes_project_id_as_one_path_segment(self) -> None:
+        requests: list[httpx.Request] = []
+
+        def handler(request: httpx.Request) -> httpx.Response:
+            requests.append(request)
+            return httpx.Response(204)
+
+        client = NocoBaseHttpClient("https://nb.example/api", "jwt", transport=httpx.MockTransport(handler))
+        client.add_attachment_ids("../project/7?#", [8])
+
+        self.assertEqual(
+            str(requests[0].url),
+            "https://nb.example/api/t_projects/..%2Fproject%2F7%3F%23/f_projects_attachments:add",
+        )
+
+    def test_attachment_association_rejects_non_scalar_project_id_before_request(self) -> None:
+        client = NocoBaseHttpClient(
+            "https://nb.example/api",
+            "jwt",
+            transport=httpx.MockTransport(lambda request: self.fail("request must not be sent")),
+        )
+
+        with self.assertRaisesRegex(ValueError, "project_id"):
+            client.add_attachment_ids({"id": 7}, [8])
+
     def test_rejects_blank_upload_credentials(self) -> None:
         with self.assertRaises(ValueError):
             NocoBaseHttpClient(" ", "jwt")

tests/test_projects.py

@@ -113,6 +113,45 @@ class ProjectServiceTests(unittest.TestCase):
         self.assertEqual(result["error"]["code"], "invalid_project_input")
         self.assertEqual(self.nb.update_calls, [])
 
+    def test_create_rejects_nested_scalar_inputs_before_writing(self) -> None:
+        cases = {
+            "name": {"name": "x"},
+            "customer_id": {"id": 3},
+            "inquiry_date": ["2026-07-12"],
+            "progress": ("SOP",),
+            "applications": ["EV"],
+            "part_numbers": ("123",),
+            "period_usage": {"annual"},
+            "details": {"note": "x"},
+        }
+
+        for field, value in cases.items():
+            with self.subTest(field=field):
+                self.setUp()
+                arguments = {"name": "x", **{field: value}}
+                result = self.service.create_project(**arguments)
+
+                self.assertEqual(result["error"]["code"], "invalid_project_input")
+                self.assertEqual(self.nb.create_calls, [])
+                self.assertEqual(self.http.add_calls, [])
+
+    def test_update_rejects_nested_project_id_before_any_client_call(self) -> None:
+        for project_id in ({"id": 41}, [41], {41}, (41,)):
+            with self.subTest(project_id=project_id):
+                self.setUp()
+                result = self.service.update_project(project_id, name="x", add_attachment_ids=[8])
+
+                self.assertEqual(result["error"]["code"], "invalid_project_input")
+                self.assertEqual(self.nb.update_calls, [])
+                self.assertEqual(self.http.add_calls, [])
+
+    def test_create_rejects_nested_attachment_id_before_any_client_call(self) -> None:
+        result = self.service.create_project(name="x", attachment_ids=[{"id": 8}])
+
+        self.assertEqual(result["error"]["code"], "invalid_project_input")
+        self.assertEqual(self.nb.create_calls, [])
+        self.assertEqual(self.http.add_calls, [])
+
     def test_update_writes_scalars_then_deduplicated_attachment_changes_in_order(self) -> None:
         result = self.service.update_project(
             41,

tests/test_server.py

@@ -2,6 +2,9 @@ import asyncio
 import unittest
 from unittest.mock import patch
 
+import httpx
+from starlette.responses import JSONResponse
+
 from nocobase_crm_mcp.server import (
     DEFAULT_HTTP_HOST,
     DEFAULT_HTTP_PATH,
@@ -77,9 +80,8 @@ class FakeProjectService:
 
 
 class FakeUploadEndpoint:
-    async def __call__(self, scope, receive, send):
-        await send({"type": "http.response.start", "status": 200, "headers": []})
-        await send({"type": "http.response.body", "body": b"uploaded"})
+    async def __call__(self, request):
+        return JSONResponse({"uploaded": True})
 
 
 class ServerTests(unittest.TestCase):
@@ -150,6 +152,25 @@ class ServerTests(unittest.TestCase):
 
         self.assertEqual(run_asgi_request(app, path="/uploads")[0]["status"], 401)
 
+    def test_upload_route_accepts_the_matching_bearer_token(self) -> None:
+        if FastMCP is None:
+            self.skipTest("MCP SDK is not installed")
+
+        server = create_server(crm_service=FakeCrmService(), readonly=True)
+
+        async def upload_endpoint(request):
+            return JSONResponse({"uploaded": True})
+
+        app = create_http_app(server, "secret", upload_endpoint=upload_endpoint)
+
+        async def request_upload() -> httpx.Response:
+            async with httpx.AsyncClient(transport=httpx.ASGITransport(app=app), base_url="http://testserver") as client:
+                return await client.post("/uploads", headers={"Authorization": "Bearer secret"})
+
+        response = asyncio.run(request_upload())
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.json(), {"uploaded": True})
+
     def test_http_write_mode_requires_upload_api_base_url(self) -> None:
         with patch.dict("os.environ", {"NOCOBASE_MCP_TOKEN": "secret"}, clear=True):
             with patch("uvicorn.run") as uvicorn_run:

tests/test_uploads.py

@@ -3,6 +3,7 @@ import unittest
 import warnings
 from pathlib import Path
 from tempfile import TemporaryDirectory
+from unittest.mock import patch
 
 import httpx
 from starlette.applications import Starlette
@@ -103,6 +104,59 @@ class UploadEndpointTests(unittest.IsolatedAsyncioTestCase):
         self.assertEqual(temp_paths, [])
         self.assertFalse(any(issubclass(warning.category, ResourceWarning) for warning in caught))
 
+    async def test_upload_rejects_non_file_form_fields(self) -> None:
+        http_client = FakeHttpClient()
+        with TemporaryDirectory() as temp_dir:
+            async with await self._client_for(http_client, temp_dir) as client:
+                response = await client.post(
+                    "/uploads",
+                    files={"file": ("quote.pdf", b"pdf", "application/pdf")},
+                    data={"description": "quote"},
+                )
+
+            self.assertEqual(response.status_code, 400)
+            self.assertEqual(response.json()["error"]["code"], "invalid_upload")
+            self.assertEqual(http_client.calls, [])
+            self.assertEqual(list(Path(temp_dir).iterdir()), [])
+
+    async def test_upload_rejects_oversized_misreported_chunked_body_before_multipart_parsing(self) -> None:
+        http_client = FakeHttpClient()
+        endpoint = UploadEndpoint(http_client, max_bytes=5, temp_dir="/tmp")
+        parsed = False
+
+        async def forbidden_form(self):
+            nonlocal parsed
+            parsed = True
+            self.fail("multipart parsing must not begin for an oversized body")
+
+        body = b"x" * (5 + 65537)
+        scope = {
+            "type": "http",
+            "method": "POST",
+            "path": "/uploads",
+            "headers": [
+                (b"content-type", b"multipart/form-data; boundary=test"),
+                (b"content-length", b"1"),
+            ],
+        }
+        messages = [
+            {"type": "http.request", "body": body[:10], "more_body": True},
+            {"type": "http.request", "body": body[10:], "more_body": False},
+        ]
+
+        async def receive():
+            return messages.pop(0)
+
+        from starlette.requests import Request
+
+        starlette_request = Request(scope, receive=receive)
+        with patch("nocobase_crm_mcp.uploads.Request.form", new=forbidden_form):
+            response = await endpoint(starlette_request)
+
+        self.assertEqual(response.status_code, 413)
+        self.assertFalse(parsed)
+        self.assertEqual(http_client.calls, [])
+
     async def test_upload_rejects_disallowed_mime_type_before_calling_nocobase(self) -> None:
         http_client = FakeHttpClient()
         with TemporaryDirectory() as temp_dir: