From 4030b8d5ee5a9dbc7a99a15d0c46d9a2811c1f37 Mon Sep 17 00:00:00 2001 From: Marcus van Elst Date: Mon, 18 May 2026 13:07:53 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20v0.4.2=20=E2=80=94=20inspect=5Fimage=20u?= =?UTF-8?q?sed=20wrong=20DSM=20parameter=20contract?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live DSM API capture revealed the actual SYNO.Docker.Image/get contract: a single JSON-encoded parameter named `identity` that accepts both `name:tag` and `sha256:` forms. The 0.4.0 code passed `name` + `tag` + `id` and was rejected by DSM with error 114. Response shape also corrected — the endpoint returns flat top-level fields (image, tag, id, digest, size, virtual_size, author, docker_version, cmd, entrypoint, env, ports, volumes), NOT the Docker-engine inspect shape with details.Config.* + RootFS.Layers that the previous implementation assumed. Layer rendering removed; digest / author / docker_version / volumes are now displayed. Pre-resolution against list_images is no longer needed — the user input goes straight into `identity` JSON-encoded. Closes #4. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 21 +++ pyproject.toml | 2 +- src/mcp_synology_container/modules/images.py | 172 +++++-------------- tests/test_modules/test_images.py | 160 ++++++++++------- uv.lock | 2 +- 5 files changed, 167 insertions(+), 190 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2696ec3..2e0b14d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,27 @@ All notable changes to this project will be documented in this file. +## [0.4.2] - 2026-05-18 + +### Fixed + +- `inspect_image` (#4): the 0.4.0 implementation called + `SYNO.Docker.Image/get` with `name` + `tag` + `id` parameters, which + DSM rejected with error 114 ("invalid parameter"). Reverse- + engineering the live API capture revealed the correct contract: + one JSON-encoded parameter named `identity` that accepts either form: + - `name:tag` (e.g. `"gitea/gitea:1.26.1"`) + - `sha256:` (e.g. `"sha256:cd21e54e..."`) + The pre-resolution lookup against `list_images` is no longer needed — + the user input is JSON-encoded and passed straight through. +- `inspect_image` response parsing: the endpoint does NOT return + `details.Config.*` / `RootFS.Layers` (the Docker-engine inspect shape + the previous code assumed). The actual top-level fields are `image`, + `tag`, `id`, `digest`, `size`, `virtual_size`, `author`, + `docker_version`, `cmd`, `entrypoint`, `env`, `ports`, `volumes`. + Layer rendering is removed (the endpoint does not surface layers); + digest, author, docker version, and volumes are now displayed. + ## [0.4.1] - 2026-05-18 ### Removed diff --git a/pyproject.toml b/pyproject.toml index 74ac4f4..20b9077 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mcp-synology-container" -version = "0.4.1" +version = "0.4.2" description = "MCP server for Synology Container Manager" requires-python = ">=3.12" dependencies = [ diff --git a/src/mcp_synology_container/modules/images.py b/src/mcp_synology_container/modules/images.py index 2c15c20..ac76de0 100644 --- a/src/mcp_synology_container/modules/images.py +++ b/src/mcp_synology_container/modules/images.py @@ -273,109 +273,51 @@ def register_images(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None: @mcp.tool() async def inspect_image(image_id: str): - """Inspect a local image by name:tag or hash — shows config, layers, env, ports.""" - # Parse name and tag using the last ":" as separator so registry-prefixed - # images (e.g. "ghcr.io/foo/bar:v1") are handled correctly. - name, sep, tag = image_id.rpartition(":") - if not sep: - name = image_id - tag = "latest" - - # Resolve the image against the local list so the user can pass either - # name:tag or a hash (full or 12-char prefix). + """Inspect a local image by name:tag or sha256 hash — shows config, env, ports.""" + # SYNO.Docker.Image/get version=1 expects the parameter "identity" + # (JSON-encoded). It accepts both `name:tag` and `sha256:` — + # no pre-resolution needed. Confirmed via DSM API capture. try: - img_data = await client.request( - "SYNO.Docker.Image", - "list", - params={"limit": "-1", "offset": "0", "show_dsm": "false"}, - ) - except Exception as e: - return f"Error inspecting image '{image_id}': {e}" - - images: list[dict[str, Any]] = img_data.get("images", []) - is_hash = image_id.startswith("sha256:") or (len(image_id) >= 12 and ":" not in image_id) - target: dict[str, Any] | None = None - - for img in images: - if is_hash: - img_hash = img.get("id", "") - if img_hash == image_id or img_hash.startswith(image_id): - target = img - break - else: - repo = img.get("repository", "") - img_tags = img.get("tags") or [] - if repo == name and tag in img_tags: - target = img - break - - if target is None: - return f"Image '{image_id}' not found locally." - - repo = target.get("repository", name) - img_tags = target.get("tags") or [tag] - display_name = f"{repo}:{img_tags[0]}" - img_hash = target.get("id", "") - - # Call SYNO.Docker.Image/get — consistent with SYNO.Docker.Container/get - # used elsewhere in this codebase. Pass both name+tag and id defensively - # so DSM can pick whichever shape it accepts. - try: - inspect_params: dict[str, Any] = { - "name": repo, - "tag": img_tags[0] if img_tags else tag, - } - if img_hash: - inspect_params["id"] = img_hash data = await client.request( "SYNO.Docker.Image", "get", - params=inspect_params, + version=1, + params={"identity": json.dumps(image_id)}, ) except Exception as e: return f"Error inspecting image '{image_id}': {e}" - # DSM may wrap the inspect blob under "details" (like SYNO.Docker.Container/get) - # or return Docker-engine-style fields at top level. Try both. - details: dict[str, Any] = data.get("details") if isinstance(data, dict) else None - if not isinstance(details, dict): - details = data if isinstance(data, dict) else {} + if not isinstance(data, dict) or not data: + return f"Image '{image_id}' not found." - # Identity — prefer fields from inspect, fall back to list_images entry - inspect_id = details.get("Id") or img_hash or "unknown" - size_val = details.get("Size", target.get("size", 0)) or 0 - size_str = _human_size(size_val) + repo = data.get("image") or "?" + tag = data.get("tag") or "?" + display_name = f"{repo}:{tag}" + img_hash = data.get("id") or "" + digest = data.get("digest") or "" + size_val = data.get("size") or 0 + virtual_size_val = data.get("virtual_size") or 0 + author = data.get("author") or "" + docker_version = data.get("docker_version") or "" + cmd = data.get("cmd") or [] + entrypoint = data.get("entrypoint") or [] + env_list: list[str] = data.get("env") or [] + ports: list[str] = data.get("ports") or [] + volumes: list[str] = data.get("volumes") or [] - # Created — DSM list_images returns a Unix int; inspect typically returns ISO string - created_field: Any = details.get("Created") or target.get("created", 0) - if isinstance(created_field, int): - created_str = _format_created(created_field) - elif isinstance(created_field, str) and created_field: - # Trim ISO timestamp to date portion if possible - created_str = created_field.split("T")[0] - else: - created_str = "unknown" - - config: dict[str, Any] = details.get("Config") or {} - env_list: list[str] = config.get("Env") or [] - exposed_ports: dict[str, Any] = config.get("ExposedPorts") or {} - entrypoint = config.get("Entrypoint") - cmd = config.get("Cmd") - working_dir = config.get("WorkingDir") or "" - labels: dict[str, Any] = config.get("Labels") or {} - - rootfs: dict[str, Any] = details.get("RootFS") or {} - layers: list[Any] = rootfs.get("Layers") or [] - - lines = [ - f"Image: {display_name}", - f" Hash: {inspect_id}", - f" Size: {size_str}", - f" Created: {created_str}", - ] - - if working_dir: - lines.append(f" Working dir: {working_dir}") + lines = [f"Image: {display_name}"] + if img_hash: + lines.append(f" Hash: {img_hash}") + if digest: + lines.append(f" Digest: {digest}") + if size_val: + lines.append(f" Size: {_human_size(size_val)}") + if virtual_size_val and virtual_size_val != size_val: + lines.append(f" Virtual: {_human_size(virtual_size_val)}") + if author: + lines.append(f" Author: {author}") + if docker_version: + lines.append(f" Docker: {docker_version}") if entrypoint: ep_str = ( @@ -383,52 +325,32 @@ def register_images(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None: if isinstance(entrypoint, list) else str(entrypoint) ) + lines.append("") lines.append(f" Entrypoint: {ep_str}") - if cmd: cmd_str = " ".join(str(x) for x in cmd) if isinstance(cmd, list) else str(cmd) + if not entrypoint: + lines.append("") lines.append(f" Cmd: {cmd_str}") - if exposed_ports: + if ports: lines.append("") - lines.append(f"Exposed ports ({len(exposed_ports)}):") - for port in exposed_ports: + lines.append(f"Exposed ports ({len(ports)}):") + for port in ports: lines.append(f" {port}") + if volumes: + lines.append("") + lines.append(f"Volumes ({len(volumes)}):") + for vol in volumes: + lines.append(f" {vol}") + if env_list: lines.append("") lines.append(f"Environment ({len(env_list)}):") for var in env_list: lines.append(f" {var}") - if layers: - lines.append("") - lines.append(f"Layers ({len(layers)}):") - for layer in layers: - # Layer may be a string hash or a dict with size info - if isinstance(layer, dict): - layer_hash = layer.get("digest") or layer.get("Id") or "" - layer_size = layer.get("size") or layer.get("Size") - short = layer_hash.split(":")[-1][:12] if layer_hash else "?" - if isinstance(layer_size, int) and layer_size > 0: - lines.append(f" {short} {_human_size(layer_size)}") - else: - lines.append(f" {short}") - else: - layer_str = str(layer) - short = layer_str.split(":")[-1][:12] if layer_str else "?" - lines.append(f" {short}") - - if labels: - label_items = list(labels.items()) - shown = label_items[:5] - lines.append("") - lines.append(f"Labels ({len(label_items)}):") - for key, value in shown: - lines.append(f" {key}={value}") - if len(label_items) > len(shown): - lines.append(f" ... and {len(label_items) - len(shown)} more") - return "\n".join(lines) @mcp.tool() diff --git a/tests/test_modules/test_images.py b/tests/test_modules/test_images.py index fdbad29..2fa32da 100644 --- a/tests/test_modules/test_images.py +++ b/tests/test_modules/test_images.py @@ -416,35 +416,31 @@ async def test_delete_image_api_error(): # ────────────────────────────────────────────────────────────────────────────── +# Authoritative DSM SYNO.Docker.Image/get response shape (v1). Top-level +# fields only — there is NO "layers" field on this endpoint, confirmed +# via API capture against the live NAS. SAMPLE_INSPECT = { - "details": { - "Id": "sha256:aaaa", - "RepoTags": ["nginx:1.24"], - "Size": 50 * 1024 * 1024, - "Created": "2024-01-01T00:00:00Z", - "Config": { - "Env": ["NGINX_VERSION=1.24", "PATH=/usr/local/sbin"], - "ExposedPorts": {"80/tcp": {}, "443/tcp": {}}, - "Entrypoint": ["/docker-entrypoint.sh"], - "Cmd": ["nginx", "-g", "daemon off;"], - "WorkingDir": "/", - "Labels": {"maintainer": "NGINX Docker Maintainers"}, - }, - "RootFS": { - "Type": "layers", - "Layers": ["sha256:layer1", "sha256:layer2"], - }, - } + "image": "nginx", + "tag": "1.24", + "id": "sha256:aaaa1234567890abcdef", + "digest": "sha256:digestabcdef", + "size": 50 * 1024 * 1024, + "virtual_size": 50 * 1024 * 1024, + "author": "NGINX Team", + "docker_version": "20.10.0", + "cmd": ["nginx", "-g", "daemon off;"], + "entrypoint": ["/docker-entrypoint.sh"], + "env": ["NGINX_VERSION=1.24", "PATH=/usr/local/sbin"], + "ports": ["80/tcp", "443/tcp"], + "volumes": ["/var/cache/nginx"], } -def _make_inspect_client(inspect_payload=None, images_payload=None): - """Build a mock DsmClient that returns SAMPLE_IMAGES for list and inspect_payload for get.""" +def _make_inspect_client(inspect_payload=None): + """Build a mock DsmClient that returns inspect_payload for SYNO.Docker.Image/get.""" client = AsyncMock() async def mock_request(api, method, **kwargs): - if api == "SYNO.Docker.Image" and method == "list": - return images_payload if images_payload is not None else SAMPLE_IMAGES if api == "SYNO.Docker.Image" and method == "get": return inspect_payload if inspect_payload is not None else SAMPLE_INSPECT return {} @@ -471,15 +467,19 @@ async def test_inspect_image_by_name_tag(): async def test_inspect_image_by_hash(): from mcp_synology_container.modules.images import register_images - # Inspect data shaped for redis (sha256:cccc) + # Inspect data shaped for redis returned by hash lookup redis_inspect = { - "details": { - "Id": "sha256:cccc", - "RepoTags": ["redis:7"], - "Size": 30 * 1024 * 1024, - "Config": {"Env": [], "ExposedPorts": {}, "Cmd": ["redis-server"]}, - "RootFS": {"Layers": ["sha256:rlayer1"]}, - } + "image": "redis", + "tag": "7", + "id": "sha256:ccccredis", + "digest": "sha256:rdigest", + "size": 30 * 1024 * 1024, + "virtual_size": 30 * 1024 * 1024, + "cmd": ["redis-server"], + "entrypoint": [], + "env": [], + "ports": ["6379/tcp"], + "volumes": [], } client = _make_inspect_client(inspect_payload=redis_inspect) mcp, tools = make_mock_mcp() @@ -490,13 +490,41 @@ async def test_inspect_image_by_hash(): @pytest.mark.asyncio -async def test_inspect_image_not_found(): +async def test_inspect_image_uses_identity_param(): + """DSM SYNO.Docker.Image/get expects parameter 'identity' (JSON-encoded), version=1. + + Using name/tag/id (the old shape) returned DSM error 114 — this test guards + against regressing to that contract. + """ + import json + from mcp_synology_container.modules.images import register_images client = _make_inspect_client() mcp, tools = make_mock_mcp() register_images(mcp, make_config(), client) + await tools["inspect_image"](image_id="nginx:1.24") + + get_calls = [ + c for c in client.request.call_args_list if c.args[:2] == ("SYNO.Docker.Image", "get") + ] + assert len(get_calls) == 1 + call = get_calls[0] + assert call.kwargs.get("version") == 1 + params = call.kwargs.get("params") or {} + assert params == {"identity": json.dumps("nginx:1.24")} + + +@pytest.mark.asyncio +async def test_inspect_image_empty_response_treated_as_not_found(): + """An empty response dict surfaces as a clean 'not found' message.""" + from mcp_synology_container.modules.images import register_images + + client = _make_inspect_client(inspect_payload={}) + mcp, tools = make_mock_mcp() + register_images(mcp, make_config(), client) + result = await tools["inspect_image"](image_id="bogus:latest") assert "not found" in result @@ -528,7 +556,7 @@ async def test_inspect_image_shows_exposed_ports(): @pytest.mark.asyncio -async def test_inspect_image_shows_layers(): +async def test_inspect_image_shows_volumes(): from mcp_synology_container.modules.images import register_images client = _make_inspect_client() @@ -536,10 +564,7 @@ async def test_inspect_image_shows_layers(): register_images(mcp, make_config(), client) result = await tools["inspect_image"](image_id="nginx:1.24") - assert "Layers" in result - # Layer hashes truncated to 12 chars after sha256: - assert "layer1" in result - assert "layer2" in result + assert "/var/cache/nginx" in result @pytest.mark.asyncio @@ -557,31 +582,42 @@ async def test_inspect_image_shows_entrypoint_cmd(): @pytest.mark.asyncio -async def test_inspect_image_registry_prefixed(): +async def test_inspect_image_shows_digest_author_docker_version(): + """Identity-block fields specific to the DSM Image/get response are surfaced.""" + from mcp_synology_container.modules.images import register_images + + client = _make_inspect_client() + mcp, tools = make_mock_mcp() + register_images(mcp, make_config(), client) + + result = await tools["inspect_image"](image_id="nginx:1.24") + assert "sha256:digestabcdef" in result + assert "NGINX Team" in result + assert "20.10.0" in result + + +@pytest.mark.asyncio +async def test_inspect_image_registry_prefixed(): + """A registry-prefixed identifier like 'ghcr.io/foo/bar:v1' is passed through verbatim + in the JSON-encoded identity parameter.""" + import json + from mcp_synology_container.modules.images import register_images - registry_images = { - "images": [ - { - "id": "sha256:dddd", - "repository": "ghcr.io/foo/bar", - "tags": ["v1"], - "size": 100 * 1024 * 1024, - "created": 1700000000, - "upgradable": False, - } - ] - } registry_inspect = { - "details": { - "Id": "sha256:dddd", - "RepoTags": ["ghcr.io/foo/bar:v1"], - "Size": 100 * 1024 * 1024, - "Config": {"Env": [], "ExposedPorts": {}, "Cmd": ["/app"]}, - "RootFS": {"Layers": ["sha256:rlayer1"]}, - } + "image": "ghcr.io/foo/bar", + "tag": "v1", + "id": "sha256:dddd", + "digest": "sha256:gdigest", + "size": 100 * 1024 * 1024, + "virtual_size": 100 * 1024 * 1024, + "cmd": ["/app"], + "entrypoint": [], + "env": [], + "ports": [], + "volumes": [], } - client = _make_inspect_client(inspect_payload=registry_inspect, images_payload=registry_images) + client = _make_inspect_client(inspect_payload=registry_inspect) mcp, tools = make_mock_mcp() register_images(mcp, make_config(), client) @@ -589,14 +625,14 @@ async def test_inspect_image_registry_prefixed(): assert "ghcr.io/foo/bar" in result assert "v1" in result - # Verify the get call used the full registry-prefixed repository name + # The full identifier (with ':') is JSON-encoded into a single string — + # registry-prefixed names are not split into name + tag. get_calls = [ c for c in client.request.call_args_list if c.args[:2] == ("SYNO.Docker.Image", "get") ] - assert get_calls, "inspect_image must call SYNO.Docker.Image/get" + assert get_calls params = get_calls[0].kwargs.get("params") or {} - assert params.get("name") == "ghcr.io/foo/bar" - assert params.get("tag") == "v1" + assert params.get("identity") == json.dumps("ghcr.io/foo/bar:v1") @pytest.mark.asyncio @@ -607,8 +643,6 @@ async def test_inspect_image_api_error(): client = AsyncMock() async def mock_request(api, method, **kwargs): - if api == "SYNO.Docker.Image" and method == "list": - return SAMPLE_IMAGES if api == "SYNO.Docker.Image" and method == "get": raise SynologyError("inspect failed", code=120) return {} diff --git a/uv.lock b/uv.lock index 6eb2e74..719fcbc 100644 --- a/uv.lock +++ b/uv.lock @@ -362,7 +362,7 @@ wheels = [ [[package]] name = "mcp-synology-container" -version = "0.4.1" +version = "0.4.2" source = { editable = "." } dependencies = [ { name = "click" },