fix: v0.2.9 — review welle 1 (C-1, C-2, M-3)
C-1: __version__ now derived from package metadata via importlib.metadata.version() so pyproject.toml is the single source of truth. Previously stuck at "0.1.0" since the initial release. C-2: Backfill CHANGELOG entries for 0.2.7 and 0.2.8 (both releases had shipped without changelog updates) and add a 0.2.9 entry covering this welle. M-3: Reject project names containing path separators or other unsafe characters before they reach _find_compose_path. Previously a name like "../../etc" could traverse out of compose_base_path when the project was not yet registered with Container Manager. Adds _validate_project_name (regex ^[a-zA-Z0-9_-]+$, applied in read_compose, update_compose, update_image_tag, update_env_var) plus parametrized tests for valid and unsafe names and one rejection test per tool. 236 tests pass. Also: ruff format autofix on three pre-existing files (cli.py, config.py, test_config.py) — cosmetic only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -2,6 +2,93 @@
|
|||||||
|
|
||||||
All notable changes to this project will be documented in this file.
|
All notable changes to this project will be documented in this file.
|
||||||
|
|
||||||
|
## [0.2.9] - 2026-05-18
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- `__version__` is now derived from package metadata via
|
||||||
|
`importlib.metadata.version()`, eliminating the drift between
|
||||||
|
`pyproject.toml` and `src/mcp_synology_container/__init__.py` (was stuck
|
||||||
|
at `0.1.0` since the initial release).
|
||||||
|
- `compose.py`: reject project names that contain path separators or other
|
||||||
|
unsafe characters before they reach `_find_compose_path`. Previously a
|
||||||
|
name like `"../../etc"` could traverse out of `compose_base_path` when
|
||||||
|
the project was not yet registered with Container Manager. The new
|
||||||
|
`_validate_project_name` helper enforces `^[a-zA-Z0-9_-]+$` and is
|
||||||
|
applied to `read_compose`, `update_compose`, `update_image_tag`, and
|
||||||
|
`update_env_var`. Addresses M-3 from the v0.2.8 review.
|
||||||
|
|
||||||
|
### Docs
|
||||||
|
|
||||||
|
- `CHANGELOG.md` backfilled for releases 0.2.7 and 0.2.8 (entries had been
|
||||||
|
missed during those releases).
|
||||||
|
|
||||||
|
## [0.2.8] - 2026-04-21
|
||||||
|
|
||||||
|
### Added
|
||||||
|
|
||||||
|
- `tests/test_dsm_client.py` — comprehensive offline test suite for
|
||||||
|
`DsmClient`:
|
||||||
|
- `_scrub_url` and `_error_message` pure helpers.
|
||||||
|
- `request()` happy-path, API-not-cached, `_sid` scrubbing in
|
||||||
|
`HTTPStatusError`, sensitive-param log masking.
|
||||||
|
- Session re-auth retry: single-retry semantics, auth-manager-absent
|
||||||
|
path, re-auth failure path, thundering-herd (login called once under
|
||||||
|
concurrent 106 responses).
|
||||||
|
- `trigger_build_stream`: SSE fire-and-forget, JSON error detection,
|
||||||
|
`ReadTimeout` swallowing, HTTP-error scrubbing.
|
||||||
|
- `upload_text` and `download_text` happy-path + error-response branches.
|
||||||
|
- `_ensure_initialized` double-checked locking and negative-cache
|
||||||
|
cooldown behavior.
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- `DsmClient._ensure_initialized`: cache failed init outcomes for 60 s so
|
||||||
|
that repeated tool calls during a credential outage (wrong password,
|
||||||
|
IP-blocked 407, DNS failure) do not keep hammering DSM. Each caller
|
||||||
|
receives the cached exception until the cooldown window expires, after
|
||||||
|
which a fresh attempt is made. Adds `INIT_ERROR_COOLDOWN` module
|
||||||
|
constant and `_init_error` / `_init_error_until` state. Addresses M4
|
||||||
|
from the 0.2.7 review.
|
||||||
|
|
||||||
|
## [0.2.7] - 2026-04-21
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- `DsmClient`: scrub `_sid` query-parameter values from URLs embedded in
|
||||||
|
`httpx.HTTPStatusError` messages so the raw DSM session ID never reaches
|
||||||
|
log output or MCP tool responses (C1).
|
||||||
|
- `DsmClient`: re-auth lock now snapshots `_sid` before entering the lock
|
||||||
|
and skips the redundant login if another task has already refreshed the
|
||||||
|
session, eliminating duplicate logins on concurrent 106/107/119
|
||||||
|
responses (M3).
|
||||||
|
- `DsmClient.trigger_build_stream`: re-instates immediate JSON-error
|
||||||
|
detection (regression from 0.2.6). Inspects the `Content-Type` header
|
||||||
|
and reads a small capped prefix of the body for `application/json`
|
||||||
|
responses to surface DSM error codes without forcing the caller into a
|
||||||
|
multi-minute polling timeout. SSE responses remain fire-and-forget
|
||||||
|
(C2/M8).
|
||||||
|
- `compose.update_env_var`: parenthesise the apply-branch match condition
|
||||||
|
so `(isinstance AND startswith) OR (entry == var_name)` no longer
|
||||||
|
evaluates the equality branch for non-string entries — aligns the apply
|
||||||
|
side with the preview-side detection logic (M1).
|
||||||
|
|
||||||
|
### Changed
|
||||||
|
|
||||||
|
- All 23 `@mcp.tool()` functions: strip `-> str` return annotations and
|
||||||
|
trim docstrings to a single line (≤100 chars). FastMCP generates an
|
||||||
|
`outputSchema` entry for every annotated tool, which roughly doubles
|
||||||
|
the `tools/list` payload size; multi-line docstrings with
|
||||||
|
`Args:`/`Returns:` sections add further bulk that Claude Desktop must
|
||||||
|
parse on every connection.
|
||||||
|
|
||||||
|
### Chore
|
||||||
|
|
||||||
|
- `uv.lock` resynced (was stale at `0.2.2`).
|
||||||
|
- `.gitignore`: exclude `.claude/` per-user Claude Code settings.
|
||||||
|
- Mechanical `ruff check --fix` + `ruff format` cleanup (import sorting,
|
||||||
|
unused-import removal). No functional change.
|
||||||
|
|
||||||
## [0.2.6] - 2026-04-21
|
## [0.2.6] - 2026-04-21
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|||||||
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
[project]
|
[project]
|
||||||
name = "mcp-synology-container"
|
name = "mcp-synology-container"
|
||||||
version = "0.2.8"
|
version = "0.2.9"
|
||||||
description = "MCP server for Synology Container Manager"
|
description = "MCP server for Synology Container Manager"
|
||||||
requires-python = ">=3.12"
|
requires-python = ">=3.12"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
|
|||||||
@@ -1,3 +1,8 @@
|
|||||||
"""MCP server for Synology Container Manager."""
|
"""MCP server for Synology Container Manager."""
|
||||||
|
|
||||||
__version__ = "0.1.0"
|
from importlib.metadata import PackageNotFoundError, version
|
||||||
|
|
||||||
|
try:
|
||||||
|
__version__ = version("mcp-synology-container")
|
||||||
|
except PackageNotFoundError:
|
||||||
|
__version__ = "0.0.0+unknown"
|
||||||
|
|||||||
@@ -291,6 +291,7 @@ def serve(config_path: str | None) -> None:
|
|||||||
# and anyio.run() ensures the correct backend context is set up.
|
# and anyio.run() ensures the correct backend context is set up.
|
||||||
# asyncio.run() can cause issues on Windows (ProactorEventLoop + anyio).
|
# asyncio.run() can cause issues on Windows (ProactorEventLoop + anyio).
|
||||||
import anyio
|
import anyio
|
||||||
|
|
||||||
anyio.run(_run_serve, config_path)
|
anyio.run(_run_serve, config_path)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -132,10 +132,7 @@ def _validate_config(raw: dict[str, Any]) -> AppConfig:
|
|||||||
"""
|
"""
|
||||||
schema_version = raw.get("schema_version")
|
schema_version = raw.get("schema_version")
|
||||||
if schema_version != CURRENT_SCHEMA_VERSION:
|
if schema_version != CURRENT_SCHEMA_VERSION:
|
||||||
msg = (
|
msg = f"Config schema_version is {schema_version!r}, expected {CURRENT_SCHEMA_VERSION}."
|
||||||
f"Config schema_version is {schema_version!r}, "
|
|
||||||
f"expected {CURRENT_SCHEMA_VERSION}."
|
|
||||||
)
|
|
||||||
raise ValueError(msg)
|
raise ValueError(msg)
|
||||||
|
|
||||||
conn_raw = raw.get("connection", {})
|
conn_raw = raw.get("connection", {})
|
||||||
|
|||||||
@@ -25,6 +25,11 @@ logger = logging.getLogger(__name__)
|
|||||||
|
|
||||||
_VOLUME_PREFIX_RE = re.compile(r"^/volume\d+")
|
_VOLUME_PREFIX_RE = re.compile(r"^/volume\d+")
|
||||||
|
|
||||||
|
# Project names are used as path components for FileStation lookups when the
|
||||||
|
# project is not yet registered with Container Manager. Restrict to a safe
|
||||||
|
# subset so a malicious name like "../../etc" cannot escape compose_base_path.
|
||||||
|
_PROJECT_NAME_RE = re.compile(r"^[a-zA-Z0-9_-]+$")
|
||||||
|
|
||||||
|
|
||||||
def _to_filestation_path(path: str) -> str:
|
def _to_filestation_path(path: str) -> str:
|
||||||
"""Strip /volumeN prefix so paths work with the FileStation API.
|
"""Strip /volumeN prefix so paths work with the FileStation API.
|
||||||
@@ -35,6 +40,22 @@ def _to_filestation_path(path: str) -> str:
|
|||||||
return _VOLUME_PREFIX_RE.sub("", path)
|
return _VOLUME_PREFIX_RE.sub("", path)
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_project_name(project_name: str) -> str | None:
|
||||||
|
"""Return an error message if project_name is unsafe, else None.
|
||||||
|
|
||||||
|
Allowed characters: letters, digits, underscore, hyphen. Anything else
|
||||||
|
(including '/', '\\', '..', whitespace, empty string) is rejected because
|
||||||
|
the name flows into FileStation path construction when the project is
|
||||||
|
not yet known to Container Manager.
|
||||||
|
"""
|
||||||
|
if not project_name or not _PROJECT_NAME_RE.match(project_name):
|
||||||
|
return (
|
||||||
|
f"Error: invalid project name {project_name!r}. "
|
||||||
|
"Allowed: letters, digits, '_' and '-' (no slashes, dots, or spaces)."
|
||||||
|
)
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
# Recognized compose file names (in priority order)
|
# Recognized compose file names (in priority order)
|
||||||
_COMPOSE_FILENAMES = [
|
_COMPOSE_FILENAMES = [
|
||||||
"docker-compose.yml",
|
"docker-compose.yml",
|
||||||
@@ -68,6 +89,8 @@ def register_compose(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None
|
|||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
async def read_compose(project_name: str):
|
async def read_compose(project_name: str):
|
||||||
"""Read the compose file (YAML) for a project."""
|
"""Read the compose file (YAML) for a project."""
|
||||||
|
if (err := _validate_project_name(project_name)) is not None:
|
||||||
|
return err
|
||||||
path = await _find_compose_path(client, config, project_name)
|
path = await _find_compose_path(client, config, project_name)
|
||||||
if path is None:
|
if path is None:
|
||||||
project = await _find_project(client, project_name)
|
project = await _find_project(client, project_name)
|
||||||
@@ -97,6 +120,8 @@ def register_compose(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None
|
|||||||
confirmed: bool = False,
|
confirmed: bool = False,
|
||||||
):
|
):
|
||||||
"""Update a service's image tag in the compose file. Requires confirmed=True."""
|
"""Update a service's image tag in the compose file. Requires confirmed=True."""
|
||||||
|
if (err := _validate_project_name(project_name)) is not None:
|
||||||
|
return err
|
||||||
path = await _find_compose_path(client, config, project_name)
|
path = await _find_compose_path(client, config, project_name)
|
||||||
if path is None:
|
if path is None:
|
||||||
return f"No compose file found for project '{project_name}'."
|
return f"No compose file found for project '{project_name}'."
|
||||||
@@ -216,6 +241,8 @@ def register_compose(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None
|
|||||||
confirmed: bool = False,
|
confirmed: bool = False,
|
||||||
):
|
):
|
||||||
"""Add or update an env var in a service's compose definition. Requires confirmed=True."""
|
"""Add or update an env var in a service's compose definition. Requires confirmed=True."""
|
||||||
|
if (err := _validate_project_name(project_name)) is not None:
|
||||||
|
return err
|
||||||
path = await _find_compose_path(client, config, project_name)
|
path = await _find_compose_path(client, config, project_name)
|
||||||
if path is None:
|
if path is None:
|
||||||
return f"No compose file found for project '{project_name}'."
|
return f"No compose file found for project '{project_name}'."
|
||||||
@@ -311,6 +338,8 @@ def register_compose(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None
|
|||||||
confirmed: bool = False,
|
confirmed: bool = False,
|
||||||
):
|
):
|
||||||
"""Replace the entire compose file with new YAML content. Requires confirmed=True."""
|
"""Replace the entire compose file with new YAML content. Requires confirmed=True."""
|
||||||
|
if (err := _validate_project_name(project_name)) is not None:
|
||||||
|
return err
|
||||||
# Validate YAML before anything else
|
# Validate YAML before anything else
|
||||||
try:
|
try:
|
||||||
parsed = yaml.safe_load(new_content)
|
parsed = yaml.safe_load(new_content)
|
||||||
|
|||||||
@@ -1,6 +1,5 @@
|
|||||||
"""Tests for config.py."""
|
"""Tests for config.py."""
|
||||||
|
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
import yaml
|
import yaml
|
||||||
|
|
||||||
|
|||||||
@@ -5,6 +5,8 @@ from unittest.mock import AsyncMock
|
|||||||
import pytest
|
import pytest
|
||||||
import yaml
|
import yaml
|
||||||
|
|
||||||
|
from mcp_synology_container.modules.compose import _validate_project_name
|
||||||
|
|
||||||
|
|
||||||
def make_mock_mcp():
|
def make_mock_mcp():
|
||||||
tools: dict = {}
|
tools: dict = {}
|
||||||
@@ -345,3 +347,121 @@ def test_extract_version_prefix():
|
|||||||
assert _extract_version_prefix("1.24") is None # no suffix
|
assert _extract_version_prefix("1.24") is None # no suffix
|
||||||
assert _extract_version_prefix("") is None
|
assert _extract_version_prefix("") is None
|
||||||
assert _extract_version_prefix("v2.0-rc1") is None # starts with 'v'
|
assert _extract_version_prefix("v2.0-rc1") is None # starts with 'v'
|
||||||
|
|
||||||
|
|
||||||
|
# ──────────────────────────────────────────────────────────────────────
|
||||||
|
# _validate_project_name — path-traversal guard
|
||||||
|
# ──────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"name",
|
||||||
|
[
|
||||||
|
"myapp",
|
||||||
|
"MyApp",
|
||||||
|
"my-app",
|
||||||
|
"my_app",
|
||||||
|
"app123",
|
||||||
|
"A",
|
||||||
|
"1",
|
||||||
|
"snake_case-and-dash_42",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_validate_project_name_accepts_safe_names(name: str) -> None:
|
||||||
|
assert _validate_project_name(name) is None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"name",
|
||||||
|
[
|
||||||
|
"", # empty
|
||||||
|
"../etc", # parent traversal
|
||||||
|
"../../etc/passwd", # multi-level traversal
|
||||||
|
"foo/../bar", # embedded traversal
|
||||||
|
"foo/bar", # forward slash
|
||||||
|
"foo\\bar", # backslash
|
||||||
|
".", # bare dot
|
||||||
|
"..", # bare dotdot
|
||||||
|
".hidden", # leading dot
|
||||||
|
"foo.bar", # dot inside (.yaml extensions, etc.)
|
||||||
|
"foo bar", # whitespace
|
||||||
|
" foo", # leading space
|
||||||
|
"foo ", # trailing space
|
||||||
|
"foo\tbar", # tab
|
||||||
|
"foo\nbar", # newline
|
||||||
|
"foo;rm", # shell metachar
|
||||||
|
"foo|bar",
|
||||||
|
"foo&bar",
|
||||||
|
"foo*bar",
|
||||||
|
"foo?bar",
|
||||||
|
"foo:bar",
|
||||||
|
"foo'bar",
|
||||||
|
'foo"bar',
|
||||||
|
"foo$bar",
|
||||||
|
"foo`bar",
|
||||||
|
"café", # non-ASCII letter
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_validate_project_name_rejects_unsafe_names(name: str) -> None:
|
||||||
|
msg = _validate_project_name(name)
|
||||||
|
assert msg is not None
|
||||||
|
assert "invalid project name" in msg
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_read_compose_rejects_traversal_name() -> None:
|
||||||
|
"""Traversal name must be rejected before any DSM call."""
|
||||||
|
from mcp_synology_container.modules.compose import register_compose
|
||||||
|
|
||||||
|
client = AsyncMock()
|
||||||
|
mcp, tools = make_mock_mcp()
|
||||||
|
register_compose(mcp, make_config(), client)
|
||||||
|
|
||||||
|
result = await tools["read_compose"]("../../etc")
|
||||||
|
assert "invalid project name" in result
|
||||||
|
client.request.assert_not_called()
|
||||||
|
client.download_text.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_update_compose_rejects_traversal_name() -> None:
|
||||||
|
"""update_compose with a traversal name must not validate YAML or upload."""
|
||||||
|
from mcp_synology_container.modules.compose import register_compose
|
||||||
|
|
||||||
|
client = AsyncMock()
|
||||||
|
mcp, tools = make_mock_mcp()
|
||||||
|
register_compose(mcp, make_config(), client)
|
||||||
|
|
||||||
|
result = await tools["update_compose"](
|
||||||
|
"foo/../bar", "services:\n web:\n image: nginx\n", confirmed=True
|
||||||
|
)
|
||||||
|
assert "invalid project name" in result
|
||||||
|
client.upload_text.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_update_image_tag_rejects_traversal_name() -> None:
|
||||||
|
from mcp_synology_container.modules.compose import register_compose
|
||||||
|
|
||||||
|
client = AsyncMock()
|
||||||
|
mcp, tools = make_mock_mcp()
|
||||||
|
register_compose(mcp, make_config(), client)
|
||||||
|
|
||||||
|
result = await tools["update_image_tag"]("foo/bar", "web", "1.25", confirmed=True)
|
||||||
|
assert "invalid project name" in result
|
||||||
|
client.upload_text.assert_not_called()
|
||||||
|
client.download_text.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_update_env_var_rejects_traversal_name() -> None:
|
||||||
|
from mcp_synology_container.modules.compose import register_compose
|
||||||
|
|
||||||
|
client = AsyncMock()
|
||||||
|
mcp, tools = make_mock_mcp()
|
||||||
|
register_compose(mcp, make_config(), client)
|
||||||
|
|
||||||
|
result = await tools["update_env_var"]("..", "web", "FOO", "bar", confirmed=True)
|
||||||
|
assert "invalid project name" in result
|
||||||
|
client.upload_text.assert_not_called()
|
||||||
|
client.download_text.assert_not_called()
|
||||||
|
|||||||
@@ -362,7 +362,7 @@ wheels = [
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "mcp-synology-container"
|
name = "mcp-synology-container"
|
||||||
version = "0.2.8"
|
version = "0.2.9"
|
||||||
source = { editable = "." }
|
source = { editable = "." }
|
||||||
dependencies = [
|
dependencies = [
|
||||||
{ name = "click" },
|
{ name = "click" },
|
||||||
|
|||||||
Reference in New Issue
Block a user