You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This commit was created on GitHub.com and signed with GitHub’s verified signature.
Fixed
publish.yml: bump pinned mcp-publisher v1.5.0 → v1.7.6 to match the new registry OIDC audience (#79) — the v0.5.1 release ran with mcp-publisher v1.5.0 (the pin in .github/actions/install-mcp-publisher/action.yml); PyPI publish succeeded but the publish-registry job failed at GitHub OIDC login with invalid audience: expected https://registry.modelcontextprotocol.io, got [mcp-registry] (HTTP 401). Root cause: the registry deployed modelcontextprotocol/registry#1229 ("auth: bind GitHub OIDC token exchange to a per-deployment audience") in v1.7.6 on 2026-04-30 — one day before our 2026-05-01 release. v1.5.0's login github-oidc flow sends audience mcp-registry; v1.7.6's flow sends audience https://registry.modelcontextprotocol.io, which is what the new registry server validates against. Bumped the action's default from v1.5.0 to v1.7.6 (and added an explanatory comment so the next bump prompt has the rationale at hand). Re-running the failed publish-registry job on the existing v0.5.1 tag won't pick up this fix because actions/checkout@v6 resolves to the tag's commit; the next release tag will exercise the fix end-to-end. v0.5.1 itself is on PyPI as expected and is the install path users actually hit; the missed registry entry is purely directory metadata.
Keyring exception handler narrows + logs root cause (#80) — closes #38. core/auth.py:147-148 previously caught a bare except Exception: with a flat logger.debug("Keyring not available."), hiding every keyring failure mode behind one generic line: locked macOS keychain (operator-actionable: unlock the keychain), keyring.errors.NoKeyringError on a headless host (signals a config issue), keyring.errors.InitError, OS-level errors on the D-Bus reach path, and genuine library bugs. Operators running mcp-synology check -v saw "Keyring not available." with no clue whether the keychain was locked, the backend was missing, or the library blew up. Narrowed to two typed handlers: except KeyringError as e (the typed-error case — covers KeyringLocked, NoKeyringError, InitError, PasswordSetError, PasswordDeleteError, and any other keyring.errors.* class) and except OSError as e (D-Bus reach errors, permission failures on the OS keychain DB). Both log at DEBUG with exc_info=True so the actual exception type, message, and traceback land in the verbose-mode output. Genuine bugs are no longer caught — they propagate up so they can be triaged. The pre-keyring D-Bus socket pre-check (auth.py:130-131) was bumped from DEBUG to INFO and rewritten with three concrete remediations (mcp-synology setup from a real desktop session, dbus-run-session wrapper, or SYNOLOGY_USERNAME / SYNOLOGY_PASSWORD / SYNOLOGY_DEVICE_ID env vars to bypass keyring entirely) — this is the most common path on Linux services launched without a desktop session, and the previous DEBUG-only log meant operators running mcp-synology check (without -v) saw a generic "no credentials" error with no breadcrumb to the root cause. Updated tests/core/test_auth.py::_no_keyring() fixture to raise keyring.errors.NoKeyringError instead of bare Exception so existing tests exercise the production-shaped error path. New TestKeyringErrorHandling (3 cases): KeyringError logged with exc_info and message text at DEBUG, OSError logged separately, and a defense-in-depth case proving a keyring blow-up doesn't block credential resolution from config/env. Strengthened TestDbusSocketMissing::test_dbus_not_set_when_socket_missing_on_linux to assert the new INFO-level remediation hint contains the socket path AND all three remediation strings. 553 unit tests pass at 96.13% coverage.
Background update-check task: log swallowed exceptions and bound the executor (#81) — closes #39. Two related gaps in server.py::SharedClientManager._bg_update_check(). (1) The (OSError, ValueError, KeyError) handler used a bare pass, so a failed PyPI check (DNS down, malformed version string in global.yaml, state-file KeyError) silently exited the background coroutine with no breadcrumb anywhere — SYNOLOGY_LOG_LEVEL=debug showed nothing. Now logs at DEBUG with exc_info=True so the actual exception type, message, and traceback land in verbose output. Update check is best-effort, so we still don't propagate; the tool flow runs fine without an update notice. (2) The loop.run_in_executor() call was unbounded. The inner urlopen(timeout=5) covered the socket, but a thread stuck after urlopen returned (pathological YAML parsing on a malformed PyPI response, slow disk on _save_global_state()) would keep this coroutine alive forever and could delay session shutdown. Now wrapped in asyncio.timeout(10) (10s outer bound, generous margin over the 5s socket timeout); on timeout _bg_update_check catches TimeoutError separately, logs at DEBUG, and exits cleanly so the user-facing tool flow is never blocked. New TestSharedClientManagerLifecycle::test_bg_update_check_timeout_logged_and_swallowed patches asyncio.timeout to a 50ms window and run_in_executor to a 5s sleep, asserting the coroutine returns normally with the expected DEBUG log. Strengthened test_bg_update_check_swallows_errors to assert the new "Update check failed" DEBUG record carries exc_info. 554 unit tests pass at 96.13% coverage.
Validate version strings on --revert <VERSION> before they reach pip install (#83) — closes #40. cli/version.py::_do_revert previously fed target_version (and the previous_version loaded from ~/.local/state/mcp-synology/global.yaml) straight into subprocess.run(["uv", "tool", "install", "--force", f"mcp-synology=={prev}"]). shell=False already neutralized command injection, but a value like --revert latest, --revert 1.2, or --revert "1.0.0; whatever" produced an opaque pip "Invalid requirement" error instead of an actionable CLI message — and a hand-corrupted previous_version field in the state file would propagate the same garbage on a no-arg --revert. New _validate_version_string() helper applies a loose PEP 440-ish regex (^\d+\.\d+\.\d+([-.]?[a-zA-Z0-9]+)*$) that accepts 0.5.1, 0.5.1-rc1, 0.5.0a1, 1.2.3.post4, etc. and rejects empty/whitespace-only input, missing patch segments, leading/trailing whitespace, latest, v-prefixes, shell metacharacters, and path traversal. On rejection it raises click.ClickException so click renders the standard Error: ... line and exits 1, matching the other CLI error paths and naming the expected format. Validation runs once at the chokepoint inside _do_revert (after prev is resolved from either source) so both the --revert <VER> and corrupt-state code paths are covered. --auto-upgrade doesn't need validation: its click Choice(["enable", "disable"]) rejects anything else, and the _do_auto_upgrade upgrade target is a literal mcp-synology@latest not a user-supplied version. Twenty new tests: 8 valid versions accepted, 12 invalid inputs rejected, plus two _do_revert regression tests verifying that an invalid explicit --revert <VER> and a corrupt state-file previous_version both raise ClickException and never reach subprocess.run. 577 unit tests pass at 96.14% coverage.
move_files and copy_files correctly handle multi-path inputs on real DSM 7.x (#85) — closes #84. Same root-cause family as #68 (delete + getinfo): DSM 7.x's SYNO.FileStation.CopyMove start does not honor the documented comma-joined multi-path format on v2 — a request with path=/a,/b is treated as a single literal path and silently no-ops while the task reports success. Symptom on move_files(paths=[a, b]): tool returns Moved 2 item(s) ... Source files have been removed. while no files actually move on disk; same shape on copy_files. Single-path moves worked in v0.5.1 because the comma-join is never constructed for a one-element list; the multi-path code path was never exercised by an integration test that asserted both source and destination state, so the silent-no-op slipped through. Fix matches PR #77's approach for delete: extract _copy_move_one_path and iterate, issuing one DSM CopyMove task per input path inside _copy_move. escape_multi_path is no longer called there. restore_from_recycle_bin routes through _copy_move for its multi-path branch, so it inherits the fix automatically. New test_multipath_uses_per_path_serial_calls in both TestCopyFiles and TestMoveFiles asserts (a) N CopyMove start requests for N paths, (b) each carries a single path with no commas, (c) all pinned to v2, (d) remove_src=true propagates per-call. New test_copy_empty_paths_returns_not_found and test_move_empty_paths_returns_not_found cover the defensive empty-list guard added with the per-path refactor. New integration tests TestMultiPathMove and TestMultiPathCopy (re-exported in tests/vdsm/test_vdsm_integration.py) create two real folders, run multi-path move/copy, and assert BOTH dest-side AND source-side state — would have caught the original #84 regression before it shipped if it had existed in v0.5.1. Strengthened TestWriteOperations::test_07_verify_move with the previously-soft "should not be in this listing" comment converted to a hard assertion (assert "original/" not in src_listing) so silent-no-op moves can't slip past dest-only checks. 581 unit tests pass at 96.19% coverage.
Bump Pygments 2.19.2 → 2.20.0 to clear ReDoS advisory GHSA-5239-wwwm-4pmq (#82) — closes Dependabot alert #3. Pygments < 2.20.0 has an inefficient regex for GUID matching that can be triggered into ReDoS by crafted input; severity Low. We pull Pygments transitively via pytest 9.0.3 (used by the test suite — pytest --color syntax-highlights traceback output), so this is a dev-dep upgrade that doesn't affect runtime. Bumped via uv lock --upgrade-package pygments; only uv.lock changed (no pyproject.toml constraint adjustment needed because pytest doesn't pin Pygments to an upper bound). 554 unit tests still pass at 96.13% coverage on the bumped lockfile; ruff/mypy clean.