fix(recover): return the file content, not a path on this host

zerto_recover_file took a dest_dir parameter, built a path from it with no
containment check, created that directory, and wrote the recovered bytes
there. It also passed the same string to the appliance as
initialDownloadPath.

On one developer's laptop that reads as "write to my own disk". The moment
this server is shared it is an arbitrary write, and a caller-supplied path
handed to another system is worse than it looks: a UNC path from a
domain-joined appliance is an outbound authentication attempt.

The parameter is gone. The destination now comes from config only, so the
caller can influence neither where bytes land nor where the ZVM mounts.

Returning a path was also useless to a remote caller. A path on this host
means nothing on theirs, so the whole recover half of the loop would have
quietly stopped working the first time this was hosted. The tool now
returns the content: text where it decodes as UTF-8, base64 otherwise,
with the byte count and a sha256 so the caller can verify what they got.

Files over max_recover_bytes (1 MiB default) are refused and pointed at
the whole-VM ladder instead of streamed through a tool result. FLR is for
a config file or a dropped directory.

Verified live against ZVM 10.9.10: jp-ubuntu returned 158 bytes and ad1
returned 164, both as text with matching content, both sessions
unmounted, and dest_dir no longer appears in the advertised tool schema.

pytest 51 passed (3 new, including one asserting the parameter set so the
caller-supplied destination cannot come back by accident).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016yVfC5nvZowoLFnEGWhLGn

src/zerto_rewind_mcp/server.py

@@ -2,6 +2,8 @@
 
 from __future__ import annotations
 
+import base64
+import hashlib
 import json
 from pathlib import Path
 from typing import Any
@@ -32,6 +34,20 @@ _catalog: MutatingCatalog | None = None
 _settings: dict[str, Any] = {}
 
 
+# Files above this are refused rather than streamed through a tool result.
+# FLR is for a config file or a dropped directory; a disk image belongs on the
+# whole-VM ladder in docs/recover-ladder.md.
+MAX_RECOVER_BYTES = 1_048_576
+
+
+def _encode_recovered(blob: bytes) -> dict[str, Any]:
+    """Text where it is text, base64 otherwise, so the caller can just use it."""
+    try:
+        return {"encoding": "text", "content": blob.decode("utf-8")}
+    except UnicodeDecodeError:
+        return {"encoding": "base64", "content": base64.b64encode(blob).decode("ascii")}
+
+
 def _dump(payload: Any) -> str:
     return json.dumps(payload, indent=2, default=str)
 
@@ -446,16 +462,20 @@ async def zerto_recover_file(
     checkpoint_identifier: str,
     guest_path: str,
     confirmed: bool = False,
-    dest_dir: str | None = None,
 ) -> str:
     """File-level recovery from a journal checkpoint. The VM stays up.
 
+    Returns the file CONTENT, not a path. The caller may be on another machine,
+    so a path on this host is of no use to them, and letting a caller choose
+    where bytes land is an arbitrary write once this server is shared.
+
     Requires confirmed=true (human yes). Cannot run during clone/test/live/EJC.
     10.9 FLR Operator role fails; use an Administrator account.
 
     Locally replicated VPGs only. FLR is performed at the VPG's recovery site,
     so a VPG replicating to a cloud ZCA must be recovered from that ZCA's API.
     guest_path is the path on the guest: /home/x/f.conf or C:\\Users\\x\\f.txt.
+    Files larger than max_recover_bytes are refused; use the whole-VM ladder.
     """
     if not confirmed:
         return _dump(
@@ -471,7 +491,10 @@ async def zerto_recover_file(
     gate = await _flr_site_gate(client, vpg_identifier)
     if not gate.get("ok"):
         return _dump(gate)
-    dest = Path(dest_dir or _settings.get("recovery_dir") or "./recovered")
+    # Server-owned, from config only. This string is also handed to the
+    # appliance as initialDownloadPath, so a caller-supplied value would let a
+    # caller point the ZVM at a path of their choosing.
+    dest = Path(_settings.get("recovery_dir") or "./recovered")
     dest.mkdir(parents=True, exist_ok=True)
     session_id: str | None = None
     before = await _live_session_ids(client)
@@ -490,16 +513,30 @@ async def zerto_recover_file(
         token = download_token_from(token_payload)
         blob = await client.fetch_download(token)
         name = Path(guest_path.replace("\\", "/")).name or "recovered.bin"
-        out_path = dest / name
+        limit = int(_settings.get("max_recover_bytes") or MAX_RECOVER_BYTES)
-        out_path.write_bytes(blob)
+        if len(blob) > limit:
-        payload = {
+            payload = {
-            "ok": True,
+                "ok": False,
-            "path": str(out_path.resolve()),
+                "too_large": True,
-            "bytes": len(blob),
+                "bytes": len(blob),
-            "session_id": session_id,
+                "limit": limit,
-            "flr_path": flr_path,
+                "message": (
-            "message": f"Wrote {len(blob)} bytes to {out_path}",
+                    f"{name} is {len(blob)} bytes, over the {limit} byte limit for "
-        }
+                    "file level recovery. Use a bounded whole-VM operation instead: "
+                    "offsite clone or failover test."
+                ),
+            }
+        else:
+            payload = {
+                "ok": True,
+                "name": name,
+                "bytes": len(blob),
+                "sha256": hashlib.sha256(blob).hexdigest(),
+                "session_id": session_id,
+                "flr_path": flr_path,
+                **_encode_recovered(blob),
+                "message": f"Recovered {len(blob)} bytes of {name} from the journal.",
+            }
     except ZertoError as exc:
         payload = {"ok": False, "session_id": session_id, "message": str(exc)}
     finally:

tests/test_recover.py

@@ -256,3 +256,44 @@ def test_flr_gate_refuses_remote_recovery_site_and_names_it():
     # must tell the operator where the operation actually lives
     assert out["recovery_site"] == "aws-zca"
     assert "aws-zca" in out["message"]
+
+
+def test_encode_recovered_text_and_binary():
+    from zerto_rewind_mcp.server import _encode_recovered
+
+    text = _encode_recovered(b"listen: 0.0.0.0:8443\n")
+    assert text["encoding"] == "text"
+    assert text["content"] == "listen: 0.0.0.0:8443\n"
+
+    binary = _encode_recovered(b"\x89PNG\r\n\x1a\n\xff\xfe")
+    assert binary["encoding"] == "base64"
+    import base64 as b64
+
+    assert b64.b64decode(binary["content"]) == b"\x89PNG\r\n\x1a\n\xff\xfe"
+
+
+def test_recover_file_takes_no_caller_destination():
+    """The caller must not choose where bytes land, nor where the ZVM mounts.
+
+    dest_dir used to be a tool parameter whose value was also passed to the
+    appliance as initialDownloadPath.
+    """
+    import inspect
+
+    from zerto_rewind_mcp.server import zerto_recover_file
+
+    params = set(inspect.signature(zerto_recover_file).parameters)
+    assert "dest_dir" not in params
+    assert params == {
+        "vpg_identifier",
+        "vm_identifier",
+        "checkpoint_identifier",
+        "guest_path",
+        "confirmed",
+    }
+
+
+def test_recover_byte_cap_is_configurable_and_has_a_default():
+    from zerto_rewind_mcp.server import MAX_RECOVER_BYTES
+
+    assert MAX_RECOVER_BYTES == 1_048_576