From 99d72466ae404a5b91024ae6ebb961c7019929c6 Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Fri, 25 Sep 2026 20:49:48 -0300 Subject: [PATCH 1/6] fix(file-tools): decide binary overwrite checks on the execution target filesystem _check_binary_document_write stat'ed the controller host only, so a binary (.sqlite/.pdf/...) that existed solely in the task's execution target (Docker/SSH/... namespace) was treated as a new file and destroyed by a plain-text write_file/patch that even reported verified:true (#122662). The existence decision now goes through one tri-state helper (_target_regular_file_state: exists/absent/unavailable) that asks the LIVE file-ops layer - the same backend the write executes on. Host-backed envs keep today's Path.is_file semantics (OSError -> proceed); other backends are probed via ShellFileOperations._probe_regular_file, whose missing/not_regular answers prove absence while every other status fails closed with a retry message. Locality comes from the live environment object (_file_ops_uses_host_paths), never env_type strings or class-name hint tables (VercelSandboxEnvironment is unclassified there). The PDF and generic binary refusal messages are unchanged verbatim; opaque-document and SQLite-sidecar unconditional refusals are untouched. _stale_overwrite_blocker keeps its host-only probe on purpose: remote reads never record a full_write_baseline (version stability requires host metadata), so converting it would refuse every remote overwrite of a file the task fully read. Tracked as follow-up. (cherry picked from commit 8b5eb26d57d7ef7d1d975de0ef5e1e8abd9a513d) --- tests/tools/test_remote_binary_write_guard.py | 279 ++++++++++++++++++ tools/file_tools_write_guards.py | 101 +++++-- 2 files changed, 358 insertions(+), 22 deletions(-) create mode 100644 tests/tools/test_remote_binary_write_guard.py diff --git a/tests/tools/test_remote_binary_write_guard.py b/tests/tools/test_remote_binary_write_guard.py new file mode 100644 index 000000000000..4fc1935fc10a --- /dev/null +++ b/tests/tools/test_remote_binary_write_guard.py @@ -0,0 +1,279 @@ +"""Binary-document overwrite guard must decide WHERE THE WRITE WILL EXECUTE. + +A text write can never produce a valid SQLite/PDF payload, so write_file/patch +refuse to overwrite an EXISTING binary document. The existence check used to +stat the CONTROLLER's disk only, so a target that existed only in the task's +execution target (Docker/SSH/... filesystem namespace) was treated as new and +destroyed (#122662). These contracts drive real registry dispatch and real +shell/file I/O: the namespace boundary is emulated by a transport stub around +``LocalEnvironment.execute`` that rewrites the host-visible view path to a +target directory — only PATHS are substituted, never probe results. +""" + +import json +import uuid +from pathlib import Path +from types import SimpleNamespace + +import pytest + +import tools.file_tools as file_tools_mod # noqa: F401 — registers the file tools +import tools.terminal_tool as terminal_tool +from tools.environments.local import LocalEnvironment +from tools.registry import registry + +_KEEP = "KEEPME line" +_BROKEN = "CHANGED line" + + +def _binary_payload(ext: str) -> bytes: + """Real-format bytes for the target file: a genuine SQLite / PDF header plus + one matchable text line (the V4A/replace edit anchor).""" + if ext == ".pdf": + # Pure-text PDF body: raw PDF syntax is text-authorable, and the + # text-vs-binary read sniff must NOT be what saves the file — only the + # write guard does. + return (b"%PDF-1.4\n1 0 obj\n<< /Type /Catalog >>\nendobj\n" + + _KEEP.encode() + b"\nend\n%%EOF\n") + return b"SQLite format 3\x00\x01\x02\x03\xff\xfe\n" + _KEEP.encode() + b"\nend\n\x00\x07tail\n" + + +class VercelSandboxEnvironment: + """Non-local backend double whose CLASS NAME no name-hint table classifies + (the real ``VercelSandboxEnvironment`` is missing from + ``_ENV_CLASS_NAME_HINTS``), so any locality decision taken from env_type + tags, scoped config or class-name hints misclassifies it as local. + + Its transport is a real ``LocalEnvironment`` bash whose commands have every + form of the host-visible view dir rewritten to the execution-target dir — + a filesystem namespace the controller cannot stat. Paths only; the shell + answers every probe for real. ``echo $HOME`` is answered with the target + dir: the execution target's home is its own namespace's, never the host's. + """ + + env_type = "vercel_sandbox" + + def __init__(self, view_dir: Path, target_dir: Path, inner: LocalEnvironment): + self.cwd = str(view_dir) + self._inner = inner + self.failure_mode = None # None | "raise" | "error" + self._home_answer = str(target_dir) + self._sub_pairs = [] + for src, dst in ((str(view_dir), str(target_dir)),): + self._sub_pairs.append((src, dst)) + self._sub_pairs.append((src.replace("\\", "/"), dst.replace("\\", "/"))) + self._sub_pairs.append((_bash_safe(src), _bash_safe(dst))) + + def _sub(self, text: str) -> str: + for src, dst in self._sub_pairs: + text = text.replace(src, dst) + return text + + def execute(self, command: str, cwd: str = "", **kwargs) -> dict: + if self.failure_mode == "raise": + raise RuntimeError("transport down") + if self.failure_mode == "error": + return {"output": "bash: transport unreachable", "returncode": 1} + if command.strip() == "echo $HOME": + return {"output": self._home_answer + "\n", "returncode": 0} + return self._inner.execute(self._sub(command), cwd=self._sub(cwd), **kwargs) + + +def _bash_safe(path: str) -> str: + from tools.environments.local import _bash_safe_path + return _bash_safe_path(path) + + +@pytest.fixture() +def remote_target(tmp_path: Path, monkeypatch): + """A task whose writes EXECUTE against ``target/`` while the controller-side + ``view/`` namespace is empty: real namespace boundary, real shell I/O.""" + view = tmp_path / "view" + target = tmp_path / "target" + view.mkdir() + target.mkdir() + # Scoped config claims 'local' — it must not decide locality (T2c pin). + monkeypatch.setattr( + terminal_tool, "_get_env_config", + lambda *a, **k: {"env_type": "local", "cwd": None}) + inner = LocalEnvironment(cwd=str(target)) + env = VercelSandboxEnvironment(view, target, inner) + task_id = f"guard-{uuid.uuid4().hex}" + # Own env key (RL/benchmark isolation override), never the shared "default". + terminal_tool.register_task_env_overrides(task_id, {"env_type": "vercel_sandbox"}) + with terminal_tool._env_lock: + terminal_tool._active_environments[task_id] = env + try: + yield SimpleNamespace(task_id=task_id, view=view, target=target, env=env) + finally: + file_tools_mod.clear_file_ops_cache(task_id) + with terminal_tool._env_lock: + terminal_tool._active_environments.pop(task_id, None) + terminal_tool._task_env_overrides.pop(task_id, None) + terminal_tool._creation_locks.pop(task_id, None) + terminal_tool._last_activity.pop(task_id, None) + + +def _dispatch(name: str, args: dict, task_id: str) -> dict: + result = registry.dispatch(name, args, task_id=task_id) + return json.loads(result) if isinstance(result, str) else result + + +def _write_file(view_path: Path, task_id: str, + content: str = "plain text replacement") -> dict: + return _dispatch("write_file", {"path": str(view_path), "content": content}, + task_id) + + +def _patch_replace(view_path: Path, task_id: str) -> dict: + return _dispatch("patch", + {"mode": "replace", "path": str(view_path), + "old_string": _KEEP, "new_string": _BROKEN}, + task_id) + + +def _patch_v4a_update(view_path: Path, task_id: str) -> dict: + patch = ("*** Begin Patch\n" + f"*** Update File: {view_path}\n" + "@@\n" + f"-{_KEEP}\n" + f"+{_BROKEN}\n" + "*** End Patch") + return _dispatch("patch", {"mode": "patch", "patch": patch}, task_id) + + +_OPERATIONS = { + "write_file": _write_file, + "patch_replace": _patch_replace, + "patch_v4a_update": _patch_v4a_update, +} + + +class TestRemoteExistingBinaryRefused: + """T1: an existing binary in the EXECUTION target is protected even though + the controller's own filesystem says the path is free.""" + + @pytest.mark.parametrize("ext", [".sqlite", ".pdf"]) + @pytest.mark.parametrize("op", sorted(_OPERATIONS)) + def test_existing_target_binary_refused_and_untouched(self, remote_target, op, ext): + view_path = remote_target.view / f"data{ext}" + target_path = remote_target.target / f"data{ext}" + target_path.write_bytes(_binary_payload(ext)) + original = target_path.read_bytes() + assert not view_path.exists(), "target must exist ONLY in the execution target" + + result = _OPERATIONS[op](view_path, remote_target.task_id) + + error = result.get("error") or "" + assert "Refusing" in error, f"{op} must refuse a remote-only binary overwrite: {result}" + if ext == ".pdf": + assert "Refusing to overwrite existing PDF" in error, error + else: + assert "Refusing to overwrite existing binary file" in error, error + assert target_path.read_bytes() == original, "target bytes must be untouched" + assert not view_path.exists(), "the controller namespace must stay clean" + + +class TestTriStateContract: + """T2: absent -> creation allowed; probe failure -> fail closed; locality + comes from the LIVE file-ops environment, never name-hint tables.""" + + @pytest.mark.parametrize("ext", [".sqlite", ".pdf"]) + def test_absent_on_target_creation_allowed(self, remote_target, ext): + view_path = remote_target.view / f"new{ext}" + content = "%PDF-1.4\n%%EOF\n" if ext == ".pdf" else "new db text fixture\n" + + result = _write_file(view_path, remote_target.task_id, content) + + assert not result.get("error"), f"creation must stay allowed: {result}" + written = (remote_target.target / f"new{ext}").read_bytes() + assert written == content.encode(), "the real write must land in the execution target" + assert not view_path.exists() + + def test_absent_on_target_v4a_add_allowed(self, remote_target): + view_path = remote_target.view / "fresh.pdf" + patch = ("*** Begin Patch\n" + f"*** Add File: {view_path}\n" + "+%PDF-1.4\n" + "+%%EOF\n" + "*** End Patch") + + result = _dispatch("patch", {"mode": "patch", "patch": patch}, remote_target.task_id) + + assert not result.get("error"), f"V4A Add of a new .pdf must stay allowed: {result}" + # V4A Add joins the '+' lines with '\n' (no trailing newline). + assert (remote_target.target / "fresh.pdf").read_bytes() == b"%PDF-1.4\n%%EOF" + + @pytest.mark.parametrize("failure_mode", ["raise", "error"]) + def test_probe_transport_failure_fails_closed(self, remote_target, failure_mode): + view_path = remote_target.view / "data.sqlite" + target_path = remote_target.target / "data.sqlite" + target_path.write_bytes(_binary_payload(".sqlite")) + original = target_path.read_bytes() + remote_target.env.failure_mode = failure_mode + + result = _write_file(view_path, remote_target.task_id) + + error = result.get("error") or "" + assert "Refusing" in error, f"an unstat-able target must fail closed: {result}" + assert "establish" in error and "retry" in error.lower(), error + assert target_path.read_bytes() == original, "target bytes must be untouched" + + def test_locality_authority_is_the_live_file_ops_env(self, remote_target): + """LOCALITY AUTHORITY pin: the backend is non-local and its env_type tag + ('vercel_sandbox') is unclassified by every name-hint table while scoped + config reports 'local'. A guard that classifies locality from env_type + strings, config or class-name hints probes the WRONG filesystem and + destroys the remote-only binary — only the live file-ops environment + object may decide.""" + view_path = remote_target.view / "data.sqlite" + target_path = remote_target.target / "data.sqlite" + target_path.write_bytes(_binary_payload(".sqlite")) + original = target_path.read_bytes() + assert remote_target.env.env_type == "vercel_sandbox" + assert not view_path.exists() + + result = _write_file(view_path, remote_target.task_id) + + error = result.get("error") or "" + assert "Refusing" in error, f"guard must probe the execution target: {result}" + assert target_path.read_bytes() == original, "target bytes must be untouched" + + +class TestResolutionFailureFallbackParity: + """T3: when task resolution fails, the guard must probe the EXACT string the + write would land on — ``write_file(_resolved or path)`` plus the file-ops + layer's own ``_expand_path``, i.e. the BACKEND's ``$HOME`` for a tilde path, + never the host's (the same wrong-filesystem bug class as #122662).""" + + def test_fallback_probe_expands_tilde_on_the_backend(self, remote_target, monkeypatch): + import tools.file_tools_write_guards as write_guards + from tools.file_tools_paths import _expand_tilde + + name = f"fallback-{uuid.uuid4().hex}.sqlite" + raw_path = f"~/{name}" + target_path = remote_target.target / name + target_path.write_bytes(_binary_payload(".sqlite")) + original = target_path.read_bytes() + # The divergence under test: the HOST tilde expansion is free while the + # write's fallback lands in the BACKEND home, where the binary lives. + assert not Path(_expand_tilde(raw_path)).exists() + assert str(remote_target.target) not in _expand_tilde(raw_path) + + def _resolver_down(*args, **kwargs): + raise OSError("forced resolution failure") + + # Both import bindings of the SAME resolver: the guard helper's and + # write_file's ``_resolve_or_none`` — in production they fail together, + # so both paths take their documented raw-string fallback. + monkeypatch.setattr(write_guards, "_resolve_path_for_task", _resolver_down) + monkeypatch.setattr(file_tools_mod, "_resolve_path_for_task", _resolver_down) + + result = _dispatch("write_file", {"path": raw_path, "content": "plain text replacement"}, + remote_target.task_id) + + error = result.get("error") or "" + assert "Refusing to overwrite existing binary file" in error, ( + f"fallback probe must hit the backend home and find the binary: {result}") + assert target_path.read_bytes() == original, "target bytes must be untouched" + assert not Path(_expand_tilde(raw_path)).exists(), "the host home must stay clean" diff --git a/tools/file_tools_write_guards.py b/tools/file_tools_write_guards.py index 3b85096f87b1..5cc1442a091f 100644 --- a/tools/file_tools_write_guards.py +++ b/tools/file_tools_write_guards.py @@ -425,12 +425,66 @@ def _check_cross_profile_path(filepath: str, task_id: str = "default") -> str | return get_container_mirror_warning(resolved, mirror_prefix=_get_container_mirror_prefix_for_task(task_id)) +def _target_regular_file_state(filepath: str, task_id: str = "default") -> str: + """``"exists"`` / ``"absent"`` / ``"unavailable"``: is a REGULAR file at + *filepath* present WHERE THE WRITE WILL EXECUTE — the task's terminal + backend, not the controller's own disk. A target that exists only inside + the Docker/SSH/... execution target must count as existing: a host-only + stat let text writes destroy remote-only binaries (#122662). + + The file-ops layer owns the filesystem view. Host-backed envs answer with + today's ``Path.is_file`` semantics (an unstat-able path counts as absent — + the pre-#122662 OSError -> proceed behaviour); every other backend is + probed through its own shell (``ShellFileOperations._probe_regular_file``), + where only ``missing``/``not_regular`` prove absence. Anything else is + ``"unavailable"``: absence not established, so callers must fail closed. + + Each branch probes the EXACT string its write path would land on, + resolution-failure fallback included: the write targets + ``write_file(_resolved or path)`` and ``write_file`` then applies its own + ``_expand_path`` (the BACKEND's ``$HOME``), so probing the host's tilde + expansion here would re-open the #122662 wrong-filesystem bug class. + """ + from tools.file_tools import _file_ops_uses_host_paths, _get_file_ops + try: + resolved = str(_resolve_path_for_task(filepath, task_id)) + except Exception: + resolved = None + try: + file_ops = _get_file_ops(task_id) + except Exception: + return "unavailable" + if _file_ops_uses_host_paths(file_ops): + # Host writes land on the resolved path, else today's HOST + # ``_expand_tilde`` fallback — probe exactly that string. + probe = _expand_tilde(filepath) if resolved is None else resolved + try: + return "exists" if Path(probe).is_file() else "absent" + except OSError: + return "absent" + try: + # Backend writes land on ``_expand_path(_resolved or path)``: the + # backend's own home for a tilde fallback, never the host's. + _size, status = file_ops._probe_regular_file(file_ops._expand_path(resolved or filepath)) + except Exception: + return "unavailable" + if status == "ok": + return "exists" + if status in ("missing", "not_regular"): + # not_regular: no REGULAR file at the path (dir/FIFO/dangling link) — + # the same answer Path.is_file gives on the host. + return "absent" + return "unavailable" + + def _check_binary_document_write(filepath: str, task_id: str = "default") -> str | None: """Reject text-tool writes that would corrupt a binary document (read_file showed EXTRACTED text, so the model may write it back). Opaque document formats and SQLite sidecars (-wal/-shm/-journal) are always rejected; .pdf and every other BINARY_EXTENSIONS suffix only when OVERWRITING an existing file (raw PDF syntax - is text-authorable and text fixtures named ``*.db`` exist). + is text-authorable and text fixtures named ``*.db`` exist). "Existing" is asked + of the filesystem the write will hit — the task's backend, via + ``_target_regular_file_state`` — never the controller's disk alone (#122662). ``read_file`` auto-extracts .docx/.xlsx/.pptx (and PDF, via anydoc) to readable text, so the model plausibly believes it holds the file's contents and tries to write the edited text back with @@ -462,28 +516,31 @@ def _check_binary_document_write(filepath: str, task_id: str = "default") -> str # syntax is text-authorable and text fixtures named ``*.db`` exist. pdf = is_pdf_path(filepath) if pdf or has_binary_extension(filepath): - try: - resolved = Path(_resolve_path_for_task(filepath, task_id)) - except Exception: - resolved = Path(_expand_tilde(filepath)) - try: - if resolved.is_file(): - if pdf: - return ( - f"Refusing to overwrite existing PDF '{filepath}' with plain text. " - "read_file showed you EXTRACTED text, not the real bytes — writing " - "text back would destroy the document. Use the pdf skill or a PDF " - "library via the terminal to modify it. (Creating a NEW .pdf file " - "is allowed.)") + state = _target_regular_file_state(filepath, task_id) + if state == "exists": + if pdf: return ( - f"Refusing to overwrite existing binary file '{filepath}' ({ext}) " - "with plain text — read_file showed you extracted or mojibake " - "text, not the real bytes, and writing text back would destroy " - "the file. Use a binary-aware tool via the terminal to modify it " - "(for SQLite databases, the sqlite3 CLI or a SQLite library). " - "(Creating a NEW file with this extension is allowed.)") - except OSError: - pass + f"Refusing to overwrite existing PDF '{filepath}' with plain text. " + "read_file showed you EXTRACTED text, not the real bytes — writing " + "text back would destroy the document. Use the pdf skill or a PDF " + "library via the terminal to modify it. (Creating a NEW .pdf file " + "is allowed.)") + return ( + f"Refusing to overwrite existing binary file '{filepath}' ({ext}) " + "with plain text — read_file showed you extracted or mojibake " + "text, not the real bytes, and writing text back would destroy " + "the file. Use a binary-aware tool via the terminal to modify it " + "(for SQLite databases, the sqlite3 CLI or a SQLite library). " + "(Creating a NEW file with this extension is allowed.)") + if state == "unavailable": + # Fail closed: absence not proven on the filesystem the write would + # hit, so proceeding could destroy a binary the guard never saw. + return ( + f"Refusing to write to '{filepath}': could not establish whether " + "the target file already exists where this write would execute " + "(the terminal environment may be starting, unreachable, or was " + "removed). The file was NOT modified — retry once the environment " + "is reachable.") return None From 6b2e9cad0e9817ff59d18cab7f4880193e1fe9d4 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 26 Sep 2026 18:23:22 +0530 Subject: [PATCH 2/6] fix(file-tools): count bad_size probe as an existing remote binary _probe_regular_file returns "bad_size" only after `[ -f ]` succeeded and `wc -c` output was unparseable, so a regular file IS present on the execution target. Treat it as "exists" (the precise existing-binary refusal) instead of the generic "unavailable" retry message; both refuse, but the retry hint is wrong for a file that is known to exist. Mirrors how the other _probe_regular_file callers treat bad_size. Idea and the original report/Docker reproduction come from #122663, the first submitted fix for this bug. Fixes #122662 Co-authored-by: liuzikaii <2319582736@qq.com> --- tools/file_tools_write_guards.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/file_tools_write_guards.py b/tools/file_tools_write_guards.py index 5cc1442a091f..0b0d0d3aa47b 100644 --- a/tools/file_tools_write_guards.py +++ b/tools/file_tools_write_guards.py @@ -468,7 +468,8 @@ def _target_regular_file_state(filepath: str, task_id: str = "default") -> str: _size, status = file_ops._probe_regular_file(file_ops._expand_path(resolved or filepath)) except Exception: return "unavailable" - if status == "ok": + if status in ("ok", "bad_size"): + # bad_size: ``[ -f ]`` succeeded, only ``wc`` was unparseable. return "exists" if status in ("missing", "not_regular"): # not_regular: no REGULAR file at the path (dir/FIFO/dangling link) — From 8fb552be36006afef1c3ad4a9d978202df2a8117 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 26 Sep 2026 18:25:15 +0530 Subject: [PATCH 3/6] test(file-tools): trim remote binary guard tests to two invariants Keep the two contracts that pin #122662: (1) write_file / patch replace / V4A update against a binary that exists only on an unhinted non-local backend (VercelSandboxEnvironment, scoped config claiming 'local') is refused with bytes untouched, for .sqlite and .pdf; (2) a probe transport failure (raise or error return) fails closed. Creation-allowed, locality and tilde-fallback cases are covered by the first invariant's setup or by tests/tools/test_binary_document_write_guard.py, and the salvage shape caps a stack at two invariant tests. --- tests/tools/test_remote_binary_write_guard.py | 108 ++---------------- 1 file changed, 9 insertions(+), 99 deletions(-) diff --git a/tests/tools/test_remote_binary_write_guard.py b/tests/tools/test_remote_binary_write_guard.py index 4fc1935fc10a..14f58abd6683 100644 --- a/tests/tools/test_remote_binary_write_guard.py +++ b/tests/tools/test_remote_binary_write_guard.py @@ -174,106 +174,16 @@ def test_existing_target_binary_refused_and_untouched(self, remote_target, op, e assert not view_path.exists(), "the controller namespace must stay clean" -class TestTriStateContract: - """T2: absent -> creation allowed; probe failure -> fail closed; locality - comes from the LIVE file-ops environment, never name-hint tables.""" - - @pytest.mark.parametrize("ext", [".sqlite", ".pdf"]) - def test_absent_on_target_creation_allowed(self, remote_target, ext): - view_path = remote_target.view / f"new{ext}" - content = "%PDF-1.4\n%%EOF\n" if ext == ".pdf" else "new db text fixture\n" - - result = _write_file(view_path, remote_target.task_id, content) - - assert not result.get("error"), f"creation must stay allowed: {result}" - written = (remote_target.target / f"new{ext}").read_bytes() - assert written == content.encode(), "the real write must land in the execution target" - assert not view_path.exists() - - def test_absent_on_target_v4a_add_allowed(self, remote_target): - view_path = remote_target.view / "fresh.pdf" - patch = ("*** Begin Patch\n" - f"*** Add File: {view_path}\n" - "+%PDF-1.4\n" - "+%%EOF\n" - "*** End Patch") - - result = _dispatch("patch", {"mode": "patch", "patch": patch}, remote_target.task_id) - - assert not result.get("error"), f"V4A Add of a new .pdf must stay allowed: {result}" - # V4A Add joins the '+' lines with '\n' (no trailing newline). - assert (remote_target.target / "fresh.pdf").read_bytes() == b"%PDF-1.4\n%%EOF" - - @pytest.mark.parametrize("failure_mode", ["raise", "error"]) - def test_probe_transport_failure_fails_closed(self, remote_target, failure_mode): - view_path = remote_target.view / "data.sqlite" - target_path = remote_target.target / "data.sqlite" - target_path.write_bytes(_binary_payload(".sqlite")) - original = target_path.read_bytes() +def test_remote_probe_failure_fails_closed(remote_target): + """An unreachable/erroring backend cannot prove absence: refuse, bytes intact.""" + target_path = remote_target.target / "data.sqlite" + target_path.write_bytes(_binary_payload(".sqlite")) + original = target_path.read_bytes() + for failure_mode in ("raise", "error"): remote_target.env.failure_mode = failure_mode - result = _write_file(view_path, remote_target.task_id) + result = _write_file(remote_target.view / "data.sqlite", remote_target.task_id) error = result.get("error") or "" - assert "Refusing" in error, f"an unstat-able target must fail closed: {result}" - assert "establish" in error and "retry" in error.lower(), error - assert target_path.read_bytes() == original, "target bytes must be untouched" - - def test_locality_authority_is_the_live_file_ops_env(self, remote_target): - """LOCALITY AUTHORITY pin: the backend is non-local and its env_type tag - ('vercel_sandbox') is unclassified by every name-hint table while scoped - config reports 'local'. A guard that classifies locality from env_type - strings, config or class-name hints probes the WRONG filesystem and - destroys the remote-only binary — only the live file-ops environment - object may decide.""" - view_path = remote_target.view / "data.sqlite" - target_path = remote_target.target / "data.sqlite" - target_path.write_bytes(_binary_payload(".sqlite")) - original = target_path.read_bytes() - assert remote_target.env.env_type == "vercel_sandbox" - assert not view_path.exists() - - result = _write_file(view_path, remote_target.task_id) - - error = result.get("error") or "" - assert "Refusing" in error, f"guard must probe the execution target: {result}" - assert target_path.read_bytes() == original, "target bytes must be untouched" - - -class TestResolutionFailureFallbackParity: - """T3: when task resolution fails, the guard must probe the EXACT string the - write would land on — ``write_file(_resolved or path)`` plus the file-ops - layer's own ``_expand_path``, i.e. the BACKEND's ``$HOME`` for a tilde path, - never the host's (the same wrong-filesystem bug class as #122662).""" - - def test_fallback_probe_expands_tilde_on_the_backend(self, remote_target, monkeypatch): - import tools.file_tools_write_guards as write_guards - from tools.file_tools_paths import _expand_tilde - - name = f"fallback-{uuid.uuid4().hex}.sqlite" - raw_path = f"~/{name}" - target_path = remote_target.target / name - target_path.write_bytes(_binary_payload(".sqlite")) - original = target_path.read_bytes() - # The divergence under test: the HOST tilde expansion is free while the - # write's fallback lands in the BACKEND home, where the binary lives. - assert not Path(_expand_tilde(raw_path)).exists() - assert str(remote_target.target) not in _expand_tilde(raw_path) - - def _resolver_down(*args, **kwargs): - raise OSError("forced resolution failure") - - # Both import bindings of the SAME resolver: the guard helper's and - # write_file's ``_resolve_or_none`` — in production they fail together, - # so both paths take their documented raw-string fallback. - monkeypatch.setattr(write_guards, "_resolve_path_for_task", _resolver_down) - monkeypatch.setattr(file_tools_mod, "_resolve_path_for_task", _resolver_down) - - result = _dispatch("write_file", {"path": raw_path, "content": "plain text replacement"}, - remote_target.task_id) - - error = result.get("error") or "" - assert "Refusing to overwrite existing binary file" in error, ( - f"fallback probe must hit the backend home and find the binary: {result}") - assert target_path.read_bytes() == original, "target bytes must be untouched" - assert not Path(_expand_tilde(raw_path)).exists(), "the host home must stay clean" + assert "Refusing" in error and "establish" in error, (failure_mode, result) + assert target_path.read_bytes() == original From 9253e50f300adf8c2da7d463a986d02a24332214 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 26 Sep 2026 21:10:59 +0530 Subject: [PATCH 4/6] docs(file-tools): trim remote-binary state helper docstring to its contract The helper docstring restated the inline why-comments (exact probe string, backend $HOME tilde fallback, which statuses prove absence). Keep only the contract callers need: three return values and that anything not proven absent is "unavailable" and must fail closed. Inline comments stay. --- tools/file_tools_write_guards.py | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/tools/file_tools_write_guards.py b/tools/file_tools_write_guards.py index 0b0d0d3aa47b..59f8e1c46779 100644 --- a/tools/file_tools_write_guards.py +++ b/tools/file_tools_write_guards.py @@ -426,24 +426,12 @@ def _check_cross_profile_path(filepath: str, task_id: str = "default") -> str | def _target_regular_file_state(filepath: str, task_id: str = "default") -> str: - """``"exists"`` / ``"absent"`` / ``"unavailable"``: is a REGULAR file at - *filepath* present WHERE THE WRITE WILL EXECUTE — the task's terminal - backend, not the controller's own disk. A target that exists only inside - the Docker/SSH/... execution target must count as existing: a host-only - stat let text writes destroy remote-only binaries (#122662). - - The file-ops layer owns the filesystem view. Host-backed envs answer with - today's ``Path.is_file`` semantics (an unstat-able path counts as absent — - the pre-#122662 OSError -> proceed behaviour); every other backend is - probed through its own shell (``ShellFileOperations._probe_regular_file``), - where only ``missing``/``not_regular`` prove absence. Anything else is - ``"unavailable"``: absence not established, so callers must fail closed. - - Each branch probes the EXACT string its write path would land on, - resolution-failure fallback included: the write targets - ``write_file(_resolved or path)`` and ``write_file`` then applies its own - ``_expand_path`` (the BACKEND's ``$HOME``), so probing the host's tilde - expansion here would re-open the #122662 wrong-filesystem bug class. + """Is a REGULAR file at *filepath* present where the write will execute + (the task's backend, not the controller's disk — #122662)? + + Returns ``"exists"``, ``"absent"`` or ``"unavailable"``. Host-backed envs + keep ``Path.is_file`` semantics; anything not proven absent is + ``"unavailable"`` and callers must fail closed. """ from tools.file_tools import _file_ops_uses_host_paths, _get_file_ops try: From 4f11f2d12aa505314d4a631dc4a4379e9b023dfe Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 26 Sep 2026 21:11:43 +0530 Subject: [PATCH 5/6] test(file-tools): drop trimming leftovers in remote binary guard double The substitution pairs were built by looping over a one-element tuple (left over from a multi-pair version), and _bash_safe() only existed to do a function-local import from a module already imported at the top. Import _bash_safe_path directly and build the three pairs as a literal list. --- tests/tools/test_remote_binary_write_guard.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/tests/tools/test_remote_binary_write_guard.py b/tests/tools/test_remote_binary_write_guard.py index 14f58abd6683..6217a4eaad11 100644 --- a/tests/tools/test_remote_binary_write_guard.py +++ b/tests/tools/test_remote_binary_write_guard.py @@ -19,7 +19,7 @@ import tools.file_tools as file_tools_mod # noqa: F401 — registers the file tools import tools.terminal_tool as terminal_tool -from tools.environments.local import LocalEnvironment +from tools.environments.local import LocalEnvironment, _bash_safe_path from tools.registry import registry _KEEP = "KEEPME line" @@ -58,11 +58,12 @@ def __init__(self, view_dir: Path, target_dir: Path, inner: LocalEnvironment): self._inner = inner self.failure_mode = None # None | "raise" | "error" self._home_answer = str(target_dir) - self._sub_pairs = [] - for src, dst in ((str(view_dir), str(target_dir)),): - self._sub_pairs.append((src, dst)) - self._sub_pairs.append((src.replace("\\", "/"), dst.replace("\\", "/"))) - self._sub_pairs.append((_bash_safe(src), _bash_safe(dst))) + src, dst = str(view_dir), str(target_dir) + self._sub_pairs = [ + (src, dst), + (src.replace("\\", "/"), dst.replace("\\", "/")), + (_bash_safe_path(src), _bash_safe_path(dst)), + ] def _sub(self, text: str) -> str: for src, dst in self._sub_pairs: @@ -79,11 +80,6 @@ def execute(self, command: str, cwd: str = "", **kwargs) -> dict: return self._inner.execute(self._sub(command), cwd=self._sub(cwd), **kwargs) -def _bash_safe(path: str) -> str: - from tools.environments.local import _bash_safe_path - return _bash_safe_path(path) - - @pytest.fixture() def remote_target(tmp_path: Path, monkeypatch): """A task whose writes EXECUTE against ``target/`` while the controller-side From b0aa4b35d3f3149f5115bef3975da83ff83daf0f Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 26 Sep 2026 21:12:55 +0530 Subject: [PATCH 6/6] test(file-tools): pin remote NEW binary-extension creation as allowed The remote guard contract only pinned refusal of an existing target-side binary. Nothing pinned the other half: a path the backend proves absent must stay writable, or a guard that over-reports "exists" (e.g. treating a "missing" probe as present) would block every new .pdf/.sqlite on Docker/SSH without a red test. Extend the kept T1 test instead of adding a function (test budget); verified it fails 6/6 under a missing->exists mutation. --- tests/tools/test_remote_binary_write_guard.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_remote_binary_write_guard.py b/tests/tools/test_remote_binary_write_guard.py index 6217a4eaad11..d8bce111a44b 100644 --- a/tests/tools/test_remote_binary_write_guard.py +++ b/tests/tools/test_remote_binary_write_guard.py @@ -147,7 +147,7 @@ def _patch_v4a_update(view_path: Path, task_id: str) -> dict: class TestRemoteExistingBinaryRefused: """T1: an existing binary in the EXECUTION target is protected even though - the controller's own filesystem says the path is free.""" + the controller's own filesystem says the path is free; a NEW one is not.""" @pytest.mark.parametrize("ext", [".sqlite", ".pdf"]) @pytest.mark.parametrize("op", sorted(_OPERATIONS)) @@ -169,6 +169,13 @@ def test_existing_target_binary_refused_and_untouched(self, remote_target, op, e assert target_path.read_bytes() == original, "target bytes must be untouched" assert not view_path.exists(), "the controller namespace must stay clean" + # A proven-absent sibling is NOT refused: creating a NEW file with a + # binary extension on the remote backend stays allowed. + created = _write_file(remote_target.view / f"new{ext}", remote_target.task_id) + assert not created.get("error"), f"creating a NEW remote {ext} must be allowed: {created}" + assert (remote_target.target / f"new{ext}").read_text() == "plain text replacement" + assert not (remote_target.view / f"new{ext}").exists() + def test_remote_probe_failure_fails_closed(remote_target): """An unreachable/erroring backend cannot prove absence: refuse, bytes intact."""