Merge b6dc05de39 into b0dc50188a

reme/steps/file_io/_file_io.py

@@ -106,15 +106,28 @@ def _decode_known_file(data: bytes, file_extension: str) -> tuple[str, str]:
 # ---------------------------------------------------------------------------
 
 
-async def read_file_safe(file_path, max_bytes: int = MAX_FILE_READ_BYTES) -> tuple[str, str]:
+async def read_file_safe(
+    file_path,
+    max_bytes: int = MAX_FILE_READ_BYTES,
+    *,
+    require_complete: bool = False,
+) -> tuple[str, str]:
     """Read file in byte mode and decode using extension-aware strategy.
 
-    Returns ``(text, encoding)``.
+    Returns ``(text, encoding)``. With ``require_complete``, reject files above
+    the byte limit or reads that leave unread bytes at the EOF check. This only
+    prevents returning a truncated bounded read for write-back; it does not
+    support oversized-file editing, provide a stable snapshot against concurrent
+    changes, or make subsequent writes atomic.
     """
     stat = await aiofiles.os.stat(str(file_path))
+    if require_complete and stat.st_size > max_bytes:
+        raise ValueError(f"file size {stat.st_size} exceeds read limit of {max_bytes} bytes")
     read_size = min(stat.st_size, max_bytes)
     async with aiofiles.open(str(file_path), "rb") as f:
         data = await f.read(read_size)
+        if require_complete and await f.read(1):
+            raise ValueError("file changed during read; refusing to return incomplete contents")
     return _decode_known_file(data, Path(file_path).suffix)
 
 

reme/steps/file_io/edit.py

@@ -9,6 +9,7 @@ from ._base import _FileIOStep
 from ._file_io import get_path_lock, read_file_safe, write_file_safe
 from ._path import _check_path_permission, NON_MD_WARNING, gate_md, resolve_path
 from ...components import R
+from ...constants import MAX_FILE_READ_BYTES
 
 
 def _replace_normalized_newlines(body: str, old: str, new: str) -> tuple[str, int]:
@@ -100,7 +101,7 @@ class EditStep(_FileIOStep):
                 return None
 
             try:
-                raw_text, encoding = await read_file_safe(target)
+                raw_text, encoding = await read_file_safe(target, max_bytes=MAX_FILE_READ_BYTES, require_complete=True)
             except Exception as e:  # pylint: disable=broad-except
                 self._fail(f"read failed: {e}", path=str(target))
                 return None

tests/unit/test_edit_read_limits.py

@@ -0,0 +1,102 @@
+"""Edits must not write back a prefix produced by a bounded file read."""
+
+import asyncio
+from functools import partial
+from types import SimpleNamespace
+
+import pytest
+
+from reme.components import ApplicationContext
+from reme.steps.file_io import _file_io, edit as edit_module
+
+
+def _small_edit_limit(monkeypatch):
+    """Exercise the real bounded reader with small files, without allocating 200 MiB."""
+    monkeypatch.setattr(edit_module, "MAX_FILE_READ_BYTES", 64, raising=False)
+    monkeypatch.setattr(edit_module, "read_file_safe", partial(_file_io.read_file_safe, max_bytes=64))
+
+
+@pytest.mark.parametrize("suffix", [".md", ".txt"])
+@pytest.mark.parametrize("encoding", ["utf-8", "utf-16"])
+def test_edit_rejects_truncated_read_without_writing(tmp_path, monkeypatch, suffix, encoding):
+    """An occurrence in the prefix must not cause the unread tail to be discarded."""
+    _small_edit_limit(monkeypatch)
+    path = tmp_path / ("note" + suffix)
+    source = ("target" + "x" * 80 + "TAIL").encode(encoding)
+    path.write_bytes(source)
+    app = ApplicationContext(workspace_dir=str(tmp_path))
+
+    async def run():
+        step = edit_module.EditStep(app_context=app)
+        await step(path=path.name, old="target", new="edited")
+        assert path.read_bytes() == source
+        assert step.context.response.success is False
+        assert "limit" in step.context.response.answer
+
+    asyncio.run(run())
+
+
+@pytest.mark.parametrize("suffix", [".md", ".txt"])
+@pytest.mark.parametrize("encoding,filler", [("utf-8", 58), ("utf-16", 25)])
+def test_edit_at_read_limit_preserves_encoding(tmp_path, monkeypatch, suffix, encoding, filler):
+    """The exact byte limit remains editable, with the existing write rules."""
+    _small_edit_limit(monkeypatch)
+    text = "target" + "x" * filler
+    source = text.encode(encoding)
+    assert len(source) == 64
+    path = tmp_path / ("note" + suffix)
+    path.write_bytes(source)
+    app = ApplicationContext(workspace_dir=str(tmp_path))
+
+    async def run():
+        step = edit_module.EditStep(app_context=app)
+        await step(path=path.name, old="target", new="edited")
+        assert step.context.response.success is True
+        expected = text.replace("target", "edited") + ("\n" if suffix == ".md" else "")
+        assert path.read_bytes() == expected.encode(encoding)
+
+    asyncio.run(run())
+
+
+def test_edit_rejects_growth_after_stat(tmp_path, monkeypatch):
+    """A stale small stat cannot turn a larger actual read into a destructive edit."""
+    _small_edit_limit(monkeypatch)
+    path = tmp_path / "note.txt"
+    source = b"target" + b"x" * 80 + b"TAIL"
+    path.write_bytes(source)
+    app = ApplicationContext(workspace_dir=str(tmp_path))
+
+    async def stale_stat(_path):
+        return SimpleNamespace(st_size=8)
+
+    monkeypatch.setattr(_file_io.aiofiles.os, "stat", stale_stat)
+
+    async def run():
+        step = edit_module.EditStep(app_context=app)
+        await step(path=path.name, old="target", new="edited")
+        assert step.context.response.success is False
+        assert "incomplete contents" in step.context.response.answer
+        assert path.read_bytes() == source
+
+    asyncio.run(run())
+
+
+@pytest.mark.parametrize("size", [0, 63, 64, 65])
+def test_read_helper_complete_mode_and_default_truncation(tmp_path, size):
+    """Ordinary limited reads keep their behavior; complete reads reject only overflow."""
+    path = tmp_path / "note.txt"
+    source = b"x" * size
+    path.write_bytes(source)
+
+    async def run():
+        text, _ = await _file_io.read_file_safe(path, max_bytes=64)
+        assert text == "x" * min(size, 64)
+        if size > 64:
+            with pytest.raises(ValueError, match="limit"):
+                await _file_io.read_file_safe(path, max_bytes=64, require_complete=True)
+        else:
+            text, _ = await _file_io.read_file_safe(path, max_bytes=64, require_complete=True)
+            assert text == "x" * size
+        assert path.read_bytes() == source
+
+    asyncio.run(run())