From 5edd0518306cb99cedd9a334889216911cfa090d Mon Sep 17 00:00:00 2001 From: Marcus van Elst Date: Mon, 13 Apr 2026 18:19:22 +0200 Subject: [PATCH] Fix delete_image: POST with images JSON array (DevTools-confirmed format) DSM Container Manager rejects name+tag and sha256 id params (error 114). Browser DevTools capture shows the correct call is: POST SYNO.Docker.Image / delete / version=1 images=[{"repository":"nouchka/sqlite3","tags":["latest"]}] Changes: - Add DsmClient.post_request() for form-encoded POST requests - delete_image now calls post_request with version=1 and the images JSON array built from the resolved repository name and tag - Remove unused docker error-code dict from _error_message() - Tests mock post_request and assert the images param content Co-Authored-By: Claude Sonnet 4.6 --- src/mcp_synology_container/dsm_client.py | 84 ++++++++++++++++++-- src/mcp_synology_container/modules/images.py | 20 +++-- tests/test_modules/test_images.py | 51 +++++++----- 3 files changed, 120 insertions(+), 35 deletions(-) diff --git a/src/mcp_synology_container/dsm_client.py b/src/mcp_synology_container/dsm_client.py index d2abdf3..1ccd47b 100644 --- a/src/mcp_synology_container/dsm_client.py +++ b/src/mcp_synology_container/dsm_client.py @@ -61,12 +61,6 @@ def _error_message(code: int, api: str = "") -> str: 407: "Too many failed login attempts — account temporarily locked", 408: "IP blocked due to excessive failed attempts", } - # Docker API codes - docker = { - 1: "Project not found", - 2: "Container not found", - } - if "Auth" in api and code in auth: return auth[code] if code in common: @@ -269,7 +263,9 @@ class DsmClient: # Log with sensitive fields masked log_params = {k: ("***" if k in _SENSITIVE_PARAMS else v) for k, v in req_params.items()} retry_tag = " (retry)" if _is_retry else "" - logger.debug("DSM GET%s: %s/%s v%d — %s", retry_tag, api, method, resolved_version, log_params) + logger.debug( + "DSM GET%s: %s/%s v%d — %s", retry_tag, api, method, resolved_version, log_params + ) resp = await http.get(url, params=req_params) resp.raise_for_status() @@ -296,6 +292,72 @@ class DsmClient: raise SynologyError(_error_message(code, api), code=code) + async def post_request( + self, + api: str, + method: str, + version: int | None = None, + params: dict[str, Any] | None = None, + ) -> dict[str, Any]: + """Make a POST (form-encoded) request to the DSM API. + + Identical semantics to request(), but sends params as a form body + instead of query-string — required by some Container Manager endpoints + (e.g. SYNO.Docker.Image/delete). + + Args: + api: DSM API name (e.g. "SYNO.Docker.Image"). + method: API method (e.g. "delete"). + version: API version. Defaults to maxVersion from API info. + params: Additional form fields. + + Returns: + Response data dict from the "data" field of the envelope. + + Raises: + SynologyError: On API errors. + """ + sys.stderr.write(f"[dsm] post_request: {api}/{method}\n") + sys.stderr.flush() + await self._ensure_initialized() + http = self._get_http() + + if api not in self._api_cache: + raise SynologyError( + f"API '{api}' not found. Call query_api_info() first.", + code=102, + ) + + info = self._api_cache[api] + resolved_version = version if version is not None else info["maxVersion"] + url = f"{self._base_url}/webapi/{info['path']}" + + form: dict[str, Any] = { + "api": api, + "version": str(resolved_version), + "method": method, + } + if params: + form.update(params) + + query_params: dict[str, str] = {} + if self._sid: + query_params["_sid"] = self._sid + + log_form = {k: ("***" if k in _SENSITIVE_PARAMS else v) for k, v in form.items()} + logger.debug("DSM POST: %s/%s v%d — %s", api, method, resolved_version, log_form) + + resp = await http.post(url, params=query_params, data=form) + resp.raise_for_status() + body = resp.json() + + if body.get("success"): + return body.get("data") or {} + + code = body.get("error", {}).get("code", 0) + logger.debug("DSM POST response: %s/%s — error code %d", api, method, code) + raise SynologyError(_error_message(code, api), code=code) + async def upload_text( self, dest_folder: str, @@ -340,7 +402,13 @@ class DsmClient: if self._sid: query_params["_sid"] = self._sid - logger.debug("DSM POST: %s/upload v%d — path=%s filename=%s", api, resolved_version, dest_folder, filename) + logger.debug( + "DSM POST: %s/upload v%d — path=%s filename=%s", + api, + resolved_version, + dest_folder, + filename, + ) encoded = content.encode("utf-8") resp = await http.post( diff --git a/src/mcp_synology_container/modules/images.py b/src/mcp_synology_container/modules/images.py index 17dedfa..a74f828 100644 --- a/src/mcp_synology_container/modules/images.py +++ b/src/mcp_synology_container/modules/images.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json import logging import sys from datetime import UTC, datetime @@ -243,17 +244,22 @@ def register_images(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None: f"Call delete_image(image_id={image_id!r}, confirmed=True) to confirm." ) - # DSM requires the sha256 image ID for deletion, not name+tag. - if not img_hash: - return f"Cannot delete '{display_name}': image ID (sha256) not found in list." - - sys.stderr.write(f"[delete_image] api=SYNO.Docker.Image method=delete id={img_hash!r}\n") + # DSM Container Manager expects a POST with version=1 and an + # "images" JSON array — confirmed via browser DevTools capture. + # Format: images=[{"repository": "nginx", "tags": ["1.24"]}] + delete_repo = repo + delete_tag = img_tags[0] if img_tags else tag + images_param = json.dumps([{"repository": delete_repo, "tags": [delete_tag]}]) + sys.stderr.write( + f"[delete_image] POST SYNO.Docker.Image/delete v1 images={images_param!r}\n" + ) sys.stderr.flush() try: - await client.request( + await client.post_request( "SYNO.Docker.Image", "delete", - params={"id": img_hash}, + version=1, + params={"images": images_param}, ) except Exception as e: code = getattr(e, "code", "?") diff --git a/tests/test_modules/test_images.py b/tests/test_modules/test_images.py index 6ee6122..a3c9b23 100644 --- a/tests/test_modules/test_images.py +++ b/tests/test_modules/test_images.py @@ -203,17 +203,18 @@ async def test_delete_image_preview(): @pytest.mark.asyncio async def test_delete_image_confirmed(): + import json + from mcp_synology_container.modules.images import register_images client = AsyncMock() + client.post_request = AsyncMock(return_value={}) async def mock_request(api, method, **kwargs): if api == "SYNO.Docker.Image" and method == "list": return SAMPLE_IMAGES if api == "SYNO.Docker.Container": return {"containers": []} - if api == "SYNO.Docker.Image" and method == "delete": - return {} return {} client.request.side_effect = mock_request @@ -225,12 +226,11 @@ async def test_delete_image_confirmed(): assert "redis:7" in result assert "freed" in result - # Delete must use the sha256 id, not name+tag - delete_call = next(c for c in client.request.call_args_list if c.args[1] == "delete") - params = delete_call.kwargs.get("params") or {} - assert params.get("id") == "sha256:cccc" - assert "name" not in params - assert "tag" not in params + # post_request must be called with images JSON param, not name/tag/id + client.post_request.assert_called_once() + params = client.post_request.call_args.kwargs.get("params", {}) + images = json.loads(params["images"]) + assert images == [{"repository": "redis", "tags": ["7"]}] @pytest.mark.asyncio @@ -238,6 +238,7 @@ async def test_delete_image_not_found(): from mcp_synology_container.modules.images import register_images client = AsyncMock() + client.post_request = AsyncMock(return_value={}) async def mock_request(api, method, **kwargs): if api == "SYNO.Docker.Image" and method == "list": @@ -250,6 +251,7 @@ async def test_delete_image_not_found(): result = await tools["delete_image"](image_id="nonexistent:latest", confirmed=True) assert "not found" in result + client.post_request.assert_not_called() @pytest.mark.asyncio @@ -257,6 +259,7 @@ async def test_delete_image_in_use_blocked(): from mcp_synology_container.modules.images import register_images client = AsyncMock() + client.post_request = AsyncMock(return_value={}) async def mock_request(api, method, **kwargs): if api == "SYNO.Docker.Image" and method == "list": @@ -272,21 +275,23 @@ async def test_delete_image_in_use_blocked(): result = await tools["delete_image"](image_id="nginx:1.24", confirmed=True) assert "Cannot delete" in result assert "my-nginx" in result + client.post_request.assert_not_called() @pytest.mark.asyncio async def test_delete_image_by_hash(): + import json + from mcp_synology_container.modules.images import register_images client = AsyncMock() + client.post_request = AsyncMock(return_value={}) async def mock_request(api, method, **kwargs): if api == "SYNO.Docker.Image" and method == "list": return SAMPLE_IMAGES if api == "SYNO.Docker.Container": return {"containers": []} - if api == "SYNO.Docker.Image" and method == "delete": - return {} return {} client.request.side_effect = mock_request @@ -297,10 +302,18 @@ async def test_delete_image_by_hash(): assert "Deleted" in result assert "redis" in result + # Verify images param uses the resolved name+tag (not the hash) + call_kwargs = client.post_request.call_args + params = call_kwargs.kwargs.get("params") or {} + images = json.loads(params.get("images", "[]")) + assert images == [{"repository": "redis", "tags": ["7"]}] + @pytest.mark.asyncio async def test_delete_image_registry_prefixed_name(): """Registry-prefixed image names (e.g. ghcr.io/foo/bar:v1) must split at last ':'.""" + import json + from mcp_synology_container.modules.images import register_images registry_images = { @@ -317,14 +330,13 @@ async def test_delete_image_registry_prefixed_name(): } client = AsyncMock() + client.post_request = AsyncMock(return_value={}) async def mock_request(api, method, **kwargs): if api == "SYNO.Docker.Image" and method == "list": return registry_images if api == "SYNO.Docker.Container": return {"containers": []} - if api == "SYNO.Docker.Image" and method == "delete": - return {} return {} client.request.side_effect = mock_request @@ -337,12 +349,11 @@ async def test_delete_image_registry_prefixed_name(): assert "Deleted" in result assert "open-webui" in result - # Verify delete was called with the sha256 ID, not name+tag - delete_call = next(c for c in client.request.call_args_list if c.args[1] == "delete") - params = delete_call.kwargs.get("params") or {} - assert params.get("id") == "sha256:dddd" - assert "name" not in params - assert "tag" not in params + # images param must use full registry-prefixed repository name + call_kwargs = client.post_request.call_args + params = call_kwargs.kwargs.get("params") or {} + images = json.loads(params.get("images", "[]")) + assert images == [{"repository": "ghcr.io/open-webui/open-webui", "tags": ["v0.8.10"]}] @pytest.mark.asyncio @@ -351,14 +362,13 @@ async def test_delete_image_api_error(): from mcp_synology_container.modules.images import register_images client = AsyncMock() + client.post_request = AsyncMock(side_effect=SynologyError("delete failed", code=114)) async def mock_request(api, method, **kwargs): if api == "SYNO.Docker.Image" and method == "list": return SAMPLE_IMAGES if api == "SYNO.Docker.Container": return {"containers": []} - if api == "SYNO.Docker.Image" and method == "delete": - raise SynologyError("delete failed", code=1) return {} client.request.side_effect = mock_request @@ -367,6 +377,7 @@ async def test_delete_image_api_error(): result = await tools["delete_image"](image_id="redis:7", confirmed=True) assert "Error" in result + assert "114" in result # ──────────────────────────────────────────────────────────────────────────────