fix(file): re-check staleness after approval to close TOCTOU window

StrReplaceFile and WriteFile validated staleness before the (unbounded) approval
prompt, then wrote the in-memory content wholesale — so an external edit landing
during approval was silently clobbered (exact-string matching ran pre-approval and
gives no write-time protection). Add a second overwrite_is_stale / _reject_if_stale
check immediately after approval, returning the same stale-read error. Regression
tests mutate the file during the approval call and assert the external change
survives.

Author: elkaix

src/pythinker_code/tools/file/replace.py

@@ -495,6 +495,20 @@ async def __call__(self, params: Params) -> ToolReturnValue:
                 if not result:
                     return result.rejection_error()
 
+            # Re-check staleness after approval: the prompt is unbounded user time
+            # during which the file can change on disk, and the write below replaces
+            # `content` wholesale (the exact-string match ran against the pre-approval
+            # read). The first check cannot cover this window; the read-cache is only
+            # refreshed after the write, so the recorded read-state is still valid.
+            if await overwrite_is_stale(self._runtime.file_read_cache, p, real_p):
+                return ToolError(
+                    message=(
+                        "File has been modified since you last read it. Read it again before "
+                        "editing it so you do not clobber the external changes."
+                    ),
+                    brief="Stale read",
+                )
+
             from pythinker_code.soul.toolset import emit_current_tool_execution_started
 
             emit_current_tool_execution_started()

src/pythinker_code/tools/file/write.py

@@ -205,6 +205,17 @@ async def __call__(self, params: Params) -> ToolReturnValue:
                 if not result:
                     return result.rejection_error()
 
+            # Re-check staleness after approval: the prompt is unbounded user time
+            # during which the file can change on disk, and the overwrite below writes
+            # params.content wholesale. The first check cannot cover this window; the
+            # read-cache is only refreshed after the write, so the read-state is valid.
+            if (
+                file_existed
+                and params.mode == "overwrite"
+                and (err := await self._reject_if_stale(p, real_p))
+            ):
+                return err
+
             from pythinker_code.soul.toolset import emit_current_tool_execution_started
 
             emit_current_tool_execution_started()

tests/tools/test_str_replace_file.py

@@ -561,6 +561,49 @@ async def test_replace_blocked_when_file_changed_since_read(
     assert "external append" in await file_path.read_text()  # external change survived
 
 
+async def test_replace_blocked_when_file_changed_during_approval(
+    read_file_tool: ReadFile, str_replace_file_tool: StrReplaceFile, temp_work_dir: HostPath
+) -> None:
+    """Stale-edit guard re-checks AFTER approval: the file can change on disk during the
+    (unbounded) approval window, and the write replaces content wholesale, so the pre-approval
+    check alone would clobber the external edit. The post-approval re-check must block it.
+
+    The approval `request` is patched to mutate the file (new size + strictly-newer mtime) then
+    approve — exercising only the second check (the first ran before the prompt, when the file
+    was still original). The edit's `old` still matches the ORIGINAL content, proving exact
+    string matching alone does not protect against the external change."""
+    import os
+    from unittest.mock import AsyncMock
+
+    from pythinker_code.soul.approval import ApprovalResult
+
+    file_path = temp_work_dir / "tracked.txt"
+    await file_path.write_text("keep ME here\n")
+    assert not (await read_file_tool(ReadParams(path=str(file_path)))).is_error
+
+    external_content = "totally different external content\n"
+
+    async def mutate_then_approve(tool_name, action, description, **kwargs):  # type: ignore[no-untyped-def]
+        # External change DURING the approval window: different size + strictly-newer mtime.
+        await file_path.write_text(external_content)
+        st = os.stat(str(file_path))
+        os.utime(str(file_path), (st.st_atime, st.st_mtime + 10))
+        return ApprovalResult(approved=True)
+
+    str_replace_file_tool._approval.request = AsyncMock(side_effect=mutate_then_approve)  # type: ignore[method-assign]
+
+    # `old="ME"` matches the ORIGINAL content (validated pre-approval), so the edit is not
+    # rejected for a missing string — only the post-approval staleness re-check can block it.
+    result = await str_replace_file_tool(
+        Params(path=str(file_path), edit=Edit(old="ME", new="YOU"))
+    )
+    assert result.is_error
+    assert "modified since" in result.message
+    # The external change survived; the tool's intended edit was NOT applied (no clobber).
+    assert await file_path.read_text() == external_content
+    assert "YOU" not in await file_path.read_text()
+
+
 async def test_replace_allowed_after_read(
     read_file_tool: ReadFile, str_replace_file_tool: StrReplaceFile, temp_work_dir: HostPath
 ) -> None:

tests/tools/test_write_file.py

@@ -69,6 +69,45 @@ async def test_overwrite_blocked_when_file_changed_since_read(
     assert "v2 external change" in await file_path.read_text()  # external change survived
 
 
+async def test_overwrite_blocked_when_file_changed_during_approval(
+    read_file_tool: ReadFile, write_file_tool: WriteFile, temp_work_dir: HostPath
+) -> None:
+    """Stale-overwrite guard re-checks AFTER approval: the file can change on disk during the
+    (unbounded) approval window, and the overwrite writes params.content wholesale, so the
+    pre-approval check alone would clobber the external edit. The post-approval re-check must
+    block it.
+
+    The approval `request` is patched to mutate the file (new size + strictly-newer mtime) then
+    approve — exercising only the second check (the first ran before the prompt, when the file
+    was still original)."""
+    import os
+    from unittest.mock import AsyncMock
+
+    from pythinker_code.soul.approval import ApprovalResult
+
+    file_path = temp_work_dir / "tracked.txt"
+    await file_path.write_text("v1 content\n")
+    assert not (await read_file_tool(ReadParams(path=str(file_path)))).is_error
+
+    external_content = "v2 external change during approval\n"
+
+    async def mutate_then_approve(tool_name, action, description, **kwargs):  # type: ignore[no-untyped-def]
+        # External change DURING the approval window: different size + strictly-newer mtime.
+        await file_path.write_text(external_content)
+        st = os.stat(str(file_path))
+        os.utime(str(file_path), (st.st_atime, st.st_mtime + 10))
+        return ApprovalResult(approved=True)
+
+    write_file_tool._approval.request = AsyncMock(side_effect=mutate_then_approve)  # type: ignore[method-assign]
+
+    result = await write_file_tool(Params(path=str(file_path), content="v3 agent overwrite\n"))
+    assert result.is_error
+    assert "modified since" in result.message
+    # The external change survived; the agent's overwrite was NOT applied (no clobber).
+    assert await file_path.read_text() == external_content
+    assert "v3 agent overwrite" not in await file_path.read_text()
+
+
 async def test_overwrite_allowed_after_read(
     read_file_tool: ReadFile, write_file_tool: WriteFile, temp_work_dir: HostPath
 ) -> None: