From 9a54fe9aeb88b8213011adfdd73f0e9b535e4f9b Mon Sep 17 00:00:00 2001
From: nesquena-hermes <agent@nesquena-hermes>
Date: Thu, 18 Jun 2026 19:22:29 +0000
Subject: [PATCH] fix(#4410): pre-check workspace leaf type via lstat + close
 fd on non-regular (Codex gate v2)

Codex re-gate found two issues in the uncapped-read fix: (1) open_anchored_fd
opens blocking O_RDONLY, which HANGS on a workspace FIFO/special file swapped in
at a regular-file path; (2) the non-regular fstat branch returned without
closing the fd (leak). Fix: lstat the symlink-resolved target BEFORE any open to
skip non-regular leaves (no blocking open on a FIFO), and close the fd on the
non-regular fstat branch. Adds a threaded FIFO-no-hang regression test.
---
 api/rollback.py                               | 21 ++++++++++++--
 .../test_rollback_diff_symlink_disclosure.py  | 28 +++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/api/rollback.py b/api/rollback.py
index 004c743c..d89dee76 100644
--- a/api/rollback.py
+++ b/api/rollback.py
@@ -169,22 +169,39 @@ def _read_workspace_text(workspace_root: Path, rel_path: str) -> str | None:
         target = safe_resolve_ws(workspace_root, rel_path)
     except (ValueError, OSError):
         return None
+    # Pre-check the leaf type WITHOUT opening it: open_anchored_fd opens with
+    # blocking O_RDONLY, which would HANG on a FIFO/special file swapped in at a
+    # regular-file path. lstat on the already-symlink-resolved target tells us the
+    # type without an open, so we never block on a non-regular leaf. (The old
+    # Path.is_file() guard likewise never opened a FIFO.)
+    try:
+        if not stat.S_ISREG(os.lstat(target).st_mode):
+            return None
+    except OSError:
+        return None
     try:
         fd = open_anchored_fd(workspace_root, target, want_dir=False)
     except (FileNotFoundError, ValueError, OSError):
         return None
+    # From here the fd is owned; make sure every path closes it exactly once.
     try:
         st = os.fstat(fd)
         if not stat.S_ISREG(st.st_mode):
+            os.close(fd)
             return None
-        with os.fdopen(fd, "rb") as fh:
-            return fh.read().decode(errors="replace")
+        fh = os.fdopen(fd, "rb")
     except OSError:
         try:
             os.close(fd)
         except OSError:
             pass
         return None
+    # os.fdopen succeeded → the file object owns the fd and will close it.
+    try:
+        with fh:
+            return fh.read().decode(errors="replace")
+    except OSError:
+        return None
 
 
 # ── Public API functions (called from routes.py) ────────────────────────────
diff --git a/tests/test_rollback_diff_symlink_disclosure.py b/tests/test_rollback_diff_symlink_disclosure.py
index 4206801b..c5bab2ad 100644
--- a/tests/test_rollback_diff_symlink_disclosure.py
+++ b/tests/test_rollback_diff_symlink_disclosure.py
@@ -125,3 +125,31 @@ def test_checkpoint_diff_renders_large_workspace_file_as_modified_not_deleted(tm
         "(the capped read_file_content path regressed this): got "
         f"{statuses.get('big.txt')!r}"
     )
+
+
+def test_checkpoint_diff_skips_workspace_fifo_without_hanging(tmp_path, monkeypatch):
+    """Regression (Codex gate): a workspace FIFO/special file at a checkpoint-
+    tracked regular-file path must NOT hang the diff (open_anchored_fd opens
+    blocking O_RDONLY, which blocks forever on a FIFO with no writer) and must
+    not leak an fd — the leaf type is pre-checked via lstat before any open.
+    """
+    import threading
+
+    workspace, ckpt_dir, checkpoint = _init_checkpoint(tmp_path, monkeypatch)
+    _commit_checkpoint_file(ckpt_dir, "pipe.txt", "checkpoint content\n")
+    # Replace the workspace-side file with a FIFO (no writer → blocking open hangs).
+    os.mkfifo(workspace / "pipe.txt")
+
+    result_box = {}
+
+    def _run():
+        result_box["r"] = rollback.get_checkpoint_diff(str(workspace), checkpoint)
+
+    t = threading.Thread(target=_run, daemon=True)
+    t.start()
+    t.join(timeout=10)
+    assert not t.is_alive(), "get_checkpoint_diff hung on a workspace FIFO"
+    # FIFO workspace side is treated as absent → checkpoint file renders as deleted,
+    # never blocks and never leaks the secret/hangs.
+    statuses = {f["file"]: f.get("status") for f in result_box["r"]["files_changed"]}
+    assert statuses.get("pipe.txt") == "deleted"