fix(update): gitignored user files also block the destructive ZIP overlay

hermes_cli/update_cmd.py

@@ -1603,7 +1603,14 @@ def _zip_overlay_block_reason(
     result = subprocess.run(
         # -uall: a user-level ``status.showUntrackedFiles = no`` git config
         # would otherwise hide untracked files and silently blind this guard.
-        git_cmd + ["status", "--porcelain", "--untracked-files=all"],
+        # --ignored=matching: gitignored files are still USER DATA the ZIP
+        # overlay would permanently delete (logs, scratch files, local data)
+        # — a .gitignore entry must not blind the guard either (#87392).
+        # ``matching`` reports an ignored directory as one ``dir/`` line
+        # instead of enumerating its contents (cheaper, same verdict for the
+        # top-level filter below). NOTE: ``--ignored=all`` is NOT a valid
+        # git mode — it exits 128 and would fail-close every ZIP update.
+        git_cmd + ["status", "--porcelain", "--untracked-files=all", "--ignored=matching"],
         cwd=root,
         capture_output=True,
         text=True,
@@ -1615,6 +1622,11 @@ def _zip_overlay_block_reason(
         suffix = f" ({detail[0]})" if detail else ""
         return f"could not check the working tree{suffix}"
     lines = [line for line in (result.stdout or "").splitlines() if line.strip()]
+    # --ignored=all reports the ZIP path's own preserved entries (venv,
+    # node_modules are gitignored on every normal install). The swap never
+    # touches those top-level entries, so they must not turn into a false
+    # dirty-tree refusal. Everything else — including ignored files — blocks.
+    lines = [line for line in lines if not _is_zip_preserved_entry_status_line(line)]
     if ignore_staging_artifacts:
         lines = [
             line for line in lines if not _is_zip_staging_artifact_status_line(line)
@@ -1625,6 +1637,33 @@ def _zip_overlay_block_reason(
 
 
 _ZIP_STAGING_ARTIFACT_SUFFIXES = (".hermes-update-staging", ".hermes-update-old")
+# Single source of truth for the top-level entries the ZIP swap preserves —
+# consumed by both the dirty-tree filter below and _update_via_zip's swap loop.
+_ZIP_PRESERVED_TOP_LEVEL = {"venv", "node_modules", ".git", ".env"}
+
+
+def _is_zip_preserved_entry_status_line(line: str) -> bool:
+    """True when every path on a porcelain status line sits under a top-level
+    entry the ZIP swap preserves.
+
+    The ``" -> "`` two-path split applies ONLY to rename/copy status codes
+    (R/C): porcelain v1 does not quote a plain filename containing spaces,
+    so an ignored file literally named ``venv -> node_modules`` on an
+    ``!!``/``??`` line must be treated as ONE path — splitting it would
+    filter it as two preserved tops and fail-open into the destructive swap.
+    Requiring EVERY path preserved keeps renames leaving a preserved dir
+    (``R venv/x -> src/x``) blocking, fail-closed.
+    """
+    status, payload = (line[:2], line[3:]) if len(line) >= 3 else ("", line)
+    is_rename = any(code in "RC" for code in status)
+    paths = payload.split(" -> ") if is_rename else [payload]
+    for path in paths:
+        top_level = (
+            path.strip().strip('"').replace("\\", "/").rstrip("/").split("/", 1)[0]
+        )
+        if top_level not in _ZIP_PRESERVED_TOP_LEVEL:
+            return False
+    return True
 
 
 def _is_zip_staging_artifact_status_line(line: str) -> bool:
@@ -1868,7 +1907,7 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo
                     break
 
         # Copy updated files over existing installation, preserving venv/node_modules/.git
-        preserve = {"venv", "node_modules", ".git", ".env"}
+        preserve = _ZIP_PRESERVED_TOP_LEVEL
         entries = [i for i in os.listdir(extracted) if i not in preserve]
 
         # Two-phase replace (#76104). Phase 1 copies every entry — directories

tests/hermes_cli/test_update_zip_fallback_guards.py

@@ -247,3 +247,110 @@ def test_recheck_still_blocks_user_files_amid_staging_artifacts(tmp_path, monkey
         tmp_path, ignore_staging_artifacts=True
     )
     assert reason is not None
+
+
+def test_zip_overlay_blocked_on_ignored_user_file(tmp_path, monkeypatch):
+    """#87392 follow-up: a gitignored file outside the preserved entries is
+    still user data the overlay would delete — it must block."""
+    (tmp_path / ".git").mkdir()
+    monkeypatch.setattr(
+        update_cmd.subprocess, "run", _porcelain_run("!! scratch/notes.local\n")
+    )
+    reason = update_cmd._zip_overlay_block_reason(tmp_path)
+    assert reason is not None
+    assert "uncommitted" in reason or "untracked" in reason
+
+
+def test_zip_overlay_flag_is_valid_against_real_git(tmp_path):
+    """The ignored-mode flag must be one real git accepts — run REAL git.
+
+    Review of the first draft caught ``--ignored=all`` (not a valid mode:
+    git exits 128 'Invalid ignored mode'), which the mocked siblings could
+    not see; with an invalid flag every ZIP update would be refused as
+    'could not check the working tree'. This test creates a real repo with
+    a real .gitignore and asserts the guard both runs clean AND still sees
+    ignored user files.
+    """
+    subprocess.run(["git", "init", "-q", str(tmp_path)], check=True)
+    (tmp_path / ".gitignore").write_text("*.local\nvenv/\n")
+    subprocess.run(
+        ["git", "-C", str(tmp_path), "add", ".gitignore"], check=True
+    )
+    subprocess.run(
+        [
+            "git", "-C", str(tmp_path),
+            "-c", "user.email=t@t", "-c", "user.name=t",
+            "commit", "-qm", "init",
+        ],
+        check=True,
+    )
+    # Clean tree: guard must pass (flag valid, no false refusal).
+    assert update_cmd._zip_overlay_block_reason(tmp_path) is None
+    # Ignored user file: guard must block.
+    (tmp_path / "data.local").write_text("x")
+    reason = update_cmd._zip_overlay_block_reason(tmp_path)
+    assert reason is not None
+    # Ignored preserved entry: still no refusal.
+    (tmp_path / "data.local").unlink()
+    (tmp_path / "venv").mkdir()
+    (tmp_path / "venv" / "lib.py").write_text("x")
+    assert update_cmd._zip_overlay_block_reason(tmp_path) is None
+
+
+def test_zip_overlay_requests_ignored_files_from_git(tmp_path, monkeypatch):
+    """The status invocation itself must carry a (valid) ignored mode."""
+    seen = {}
+
+    def capture_run(cmd, **kwargs):
+        joined = " ".join(str(c) for c in cmd)
+        if "status" in joined and "--porcelain" in joined:
+            seen["cmd"] = cmd
+            return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+        return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+    (tmp_path / ".git").mkdir()
+    monkeypatch.setattr(update_cmd.subprocess, "run", capture_run)
+    update_cmd._zip_overlay_block_reason(tmp_path)
+    assert "--ignored=matching" in [str(c) for c in seen["cmd"]]
+
+
+def test_preserved_filter_does_not_split_non_rename_lines():
+    """A plain ignored FILE literally named 'venv -> node_modules' is ONE
+    path (porcelain v1 doesn't quote spaces) — splitting it would filter it
+    as two preserved tops and fail-open into the destructive swap."""
+    assert not update_cmd._is_zip_preserved_entry_status_line(
+        "!! venv -> node_modules"
+    )
+    assert not update_cmd._is_zip_preserved_entry_status_line(
+        "?? venv -> node_modules"
+    )
+    # Real rename crossing out of a preserved dir still blocks…
+    assert not update_cmd._is_zip_preserved_entry_status_line(
+        "R  venv/x -> src/x"
+    )
+    # …and a rename fully inside preserved entries is still filtered.
+    assert update_cmd._is_zip_preserved_entry_status_line(
+        "R  venv/a -> node_modules/b"
+    )
+
+
+def test_swap_preserve_set_is_the_module_constant():
+    """The swap loop and the dirty-tree filter must share one source of
+    truth for the preserved entries (no comment-synced duplicate)."""
+    import inspect
+
+    src = inspect.getsource(update_cmd._update_via_zip)
+    assert "preserve = _ZIP_PRESERVED_TOP_LEVEL" in src
+
+
+def test_zip_overlay_allows_ignored_preserved_entries(tmp_path, monkeypatch):
+    """venv/node_modules are gitignored on every normal install and the swap
+    preserves them — the ignored probe must not turn them into a false
+    refusal."""
+    (tmp_path / ".git").mkdir()
+    monkeypatch.setattr(
+        update_cmd.subprocess,
+        "run",
+        _porcelain_run("!! venv/\n!! node_modules/\n!! .env\n"),
+    )
+    assert update_cmd._zip_overlay_block_reason(tmp_path) is None