From ee7d8c56c71c12752c3ee7dd384480119aaa83c5 Mon Sep 17 00:00:00 2001
From: 0xbyt4 <35742124+0xbyt4@users.noreply.github.com>
Date: Sat, 7 Mar 2026 19:27:23 +0300
Subject: [PATCH] fix: prevent data loss in clipboard PNG conversion when
 ImageMagick fails

_convert_to_png() renamed the original file to .bmp before calling
ImageMagick convert, then unconditionally deleted the .bmp regardless
of whether convert succeeded. If convert failed, both files were gone.

- Only delete .bmp after confirmed successful conversion
- Restore original file on convert failure, timeout, or missing binary
- Add 3 tests covering failure, not-installed, and timeout scenarios
---
 hermes_cli/clipboard.py       | 11 +++++++--
 tests/tools/test_clipboard.py | 45 +++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/hermes_cli/clipboard.py b/hermes_cli/clipboard.py
index fa750d85..893a84d3 100644
--- a/hermes_cli/clipboard.py
+++ b/hermes_cli/clipboard.py
@@ -285,20 +285,27 @@ def _convert_to_png(path: Path) -> bool:
         logger.debug("Pillow BMP→PNG conversion failed: %s", e)
 
     # Fall back to ImageMagick convert
+    tmp = path.with_suffix(".bmp")
     try:
-        tmp = path.with_suffix(".bmp")
         path.rename(tmp)
         r = subprocess.run(
             ["convert", str(tmp), "png:" + str(path)],
             capture_output=True, timeout=5,
         )
-        tmp.unlink(missing_ok=True)
         if r.returncode == 0 and path.exists() and path.stat().st_size > 0:
+            tmp.unlink(missing_ok=True)
             return True
+        else:
+            # Convert failed — restore the original file
+            tmp.rename(path)
     except FileNotFoundError:
         logger.debug("ImageMagick not installed — cannot convert BMP to PNG")
+        if not path.exists() and tmp.exists():
+            tmp.rename(path)
     except Exception as e:
         logger.debug("ImageMagick BMP→PNG conversion failed: %s", e)
+        if not path.exists() and tmp.exists():
+            tmp.rename(path)
 
     # Can't convert — BMP is still usable as-is for most APIs
     return path.exists() and path.stat().st_size > 0
diff --git a/tests/tools/test_clipboard.py b/tests/tools/test_clipboard.py
index 1fb1a39e..dc064e6c 100644
--- a/tests/tools/test_clipboard.py
+++ b/tests/tools/test_clipboard.py
@@ -559,6 +559,51 @@ class TestConvertToPng:
             # (raw BMP is better than nothing)
             assert dest.exists() and dest.stat().st_size > 0
 
+    def test_imagemagick_failure_preserves_original(self, tmp_path):
+        """When ImageMagick convert fails, the original file must not be lost."""
+        dest = tmp_path / "img.png"
+        original_data = FAKE_BMP
+        dest.write_bytes(original_data)
+
+        def fake_run_fail(cmd, **kw):
+            # Simulate convert failing without producing output
+            return MagicMock(returncode=1)
+
+        with patch.dict(sys.modules, {"PIL": None, "PIL.Image": None}):
+            with patch("hermes_cli.clipboard.subprocess.run", side_effect=fake_run_fail):
+                _convert_to_png(dest)
+
+        # Original file must still exist with original content
+        assert dest.exists(), "Original file was lost after failed conversion"
+        assert dest.read_bytes() == original_data
+
+    def test_imagemagick_not_installed_preserves_original(self, tmp_path):
+        """When ImageMagick is not installed, the original file must not be lost."""
+        dest = tmp_path / "img.png"
+        original_data = FAKE_BMP
+        dest.write_bytes(original_data)
+
+        with patch.dict(sys.modules, {"PIL": None, "PIL.Image": None}):
+            with patch("hermes_cli.clipboard.subprocess.run", side_effect=FileNotFoundError):
+                _convert_to_png(dest)
+
+        assert dest.exists(), "Original file was lost when ImageMagick not installed"
+        assert dest.read_bytes() == original_data
+
+    def test_imagemagick_timeout_preserves_original(self, tmp_path):
+        """When ImageMagick times out, the original file must not be lost."""
+        import subprocess
+        dest = tmp_path / "img.png"
+        original_data = FAKE_BMP
+        dest.write_bytes(original_data)
+
+        with patch.dict(sys.modules, {"PIL": None, "PIL.Image": None}):
+            with patch("hermes_cli.clipboard.subprocess.run", side_effect=subprocess.TimeoutExpired("convert", 5)):
+                _convert_to_png(dest)
+
+        assert dest.exists(), "Original file was lost after timeout"
+        assert dest.read_bytes() == original_data
+
 
 # ── has_clipboard_image dispatch ─────────────────────────────────────────