diff --git a/tests/tools/test_binary_db_write_guard.py b/tests/tools/test_binary_db_write_guard.py new file mode 100644 index 0000000000000..0a57da372d8a1 --- /dev/null +++ b/tests/tools/test_binary_db_write_guard.py @@ -0,0 +1,125 @@ +"""Tests for the database/binary write guard extension. + +A plain-text write can never produce a valid SQLite database, and the text +tools' read->modify->write round-trip re-encodes binary bytes lossily (the +terminal transport decodes stdout with errors="replace"). write_file/patch +must therefore refuse db-family targets (.db/.sqlite/.sqlite3 plus the +-wal/-shm/-journal sidecars, whose suffixed extension defeats a plain +BINARY_EXTENSIONS lookup) and refuse OVERWRITING other existing binary +files — while new-file creation with those extensions stays allowed. +""" + +import json +import sqlite3 +from pathlib import Path + +from tools.file_tools import patch_tool, write_file_tool +from tools.file_tools_write_guards import ( + _SQLITE_DB_FAMILY_RE, + _check_binary_document_write, +) + + +def _make_minimal_db(path: Path) -> None: + conn = sqlite3.connect(path) + conn.execute("CREATE TABLE t (id INTEGER, name TEXT)") + conn.execute("INSERT INTO t VALUES (1, 'alpha')") + conn.commit() + conn.close() + + +class TestRegex: + def test_main_db_names(self): + for name in ("kanban.db", "app.sqlite", "db.sqlite3", "DATA.DB"): + assert _SQLITE_DB_FAMILY_RE.search(name), name + + def test_sidecar_names(self): + # .db-wal splits to ".db-wal" which is NOT in BINARY_EXTENSIONS — + # the hole this regex closes. + for name in ("kanban.db-wal", "kanban.db-shm", "kanban.db-journal", + "app.sqlite-wal", "x.sqlite3-shm"): + assert _SQLITE_DB_FAMILY_RE.search(name), name + + def test_non_db_names(self): + for name in ("notes.txt", "main.py", "hermes.dbk", "mongodb.txt", + "nodotfile", "a.dbx"): + assert not _SQLITE_DB_FAMILY_RE.search(name), name + + +class TestCheckBinaryDocumentWrite: + def test_db_always_rejected(self, tmp_path: Path): + # Even a NON-existing .db is rejected — text can never be a database. + err = _check_binary_document_write(str(tmp_path / "fresh.db")) + assert err is not None + assert "database" in err.lower() + + def test_wal_sidecar_rejected(self, tmp_path: Path): + err = _check_binary_document_write(str(tmp_path / "kanban.db-wal")) + assert err is not None + assert "database" in err.lower() + + def test_plain_text_allowed(self, tmp_path: Path): + assert _check_binary_document_write(str(tmp_path / "notes.txt")) is None + + def test_new_binary_file_allowed(self, tmp_path: Path): + # New-file creation with a non-db binary extension stays allowed + # (mirrors the new-.pdf rule): a text file named .dat is odd but + # harmless; the corruption argument only applies to overwrites. + assert _check_binary_document_write(str(tmp_path / "new.png")) is None + + +class TestWriteFileTool: + def test_write_file_rejects_existing_db(self, tmp_path: Path): + db = tmp_path / "kanban.db" + _make_minimal_db(db) + original = db.read_bytes() + + result = json.loads(write_file_tool(str(db), "CREATE TABLE x(y);")) + + assert result.get("error"), "text write into .db must be refused" + assert db.read_bytes() == original, "database bytes must be untouched" + # And still a valid database. + conn = sqlite3.connect(db) + assert conn.execute("PRAGMA integrity_check;").fetchone()[0] == "ok" + conn.close() + + def test_write_file_rejects_new_db_creation(self, tmp_path: Path): + fresh = tmp_path / "fresh.db" + result = json.loads(write_file_tool(str(fresh), "CREATE TABLE x(y);")) + assert result.get("error"), "new .db via text write must be refused" + assert not fresh.exists(), "no file may be created" + + def test_write_file_rejects_existing_binary_overwrite(self, tmp_path: Path): + img = tmp_path / "logo.png" + img.write_bytes(b"\\x89PNG\\r\\n\\x1a\\n" + b"\\x00" * 64) + original = img.read_bytes() + result = json.loads(write_file_tool(str(img), "not an image")) + assert result.get("error"), "binary overwrite must be refused" + assert img.read_bytes() == original + + +class TestPatchTool: + def test_patch_rejects_existing_db(self, tmp_path: Path): + db = tmp_path / "kanban.db" + _make_minimal_db(db) + original = db.read_bytes() + + result = json.loads( + patch_tool(mode="replace", path=str(db), + old_string="alpha", new_string="beta")) + + assert result.get("error"), "patch into .db must be refused" + assert db.read_bytes() == original, "database bytes must be untouched" + conn = sqlite3.connect(db) + assert conn.execute("PRAGMA integrity_check;").fetchone()[0] == "ok" + conn.close() + + def test_patch_rejects_wal_sidecar(self, tmp_path: Path): + wal = tmp_path / "kanban.db-wal" + wal.write_bytes(b"\\x00\\x01\\x02\\x00data") + original = wal.read_bytes() + result = json.loads( + patch_tool(mode="replace", path=str(wal), + old_string="data", new_string="DATA")) + assert result.get("error"), "patch into .db-wal must be refused" + assert wal.read_bytes() == original diff --git a/tools/file_tools_write_guards.py b/tools/file_tools_write_guards.py index 20acc57487ff0..fddadd11ad133 100644 --- a/tools/file_tools_write_guards.py +++ b/tools/file_tools_write_guards.py @@ -12,11 +12,16 @@ import fnmatch import os +import re from pathlib import Path from agent.file_safety import get_nt_namespace_error from tools import file_state -from tools.binary_extensions import has_opaque_document_extension, is_pdf_path +from tools.binary_extensions import ( + has_binary_extension, + has_opaque_document_extension, + is_pdf_path, +) from tools.file_tools_paths import _expand_tilde, _resolve_path_for_task from tools.file_tools_read_tracking import _has_full_write_baseline, _read_mtime_drifted @@ -415,6 +420,16 @@ 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)) +# SQLite files and their journal sidecars. ``os.path.splitext("x.db-wal")`` +# yields ".db-wal" — NOT in BINARY_EXTENSIONS — so the sidecars of every +# WAL-mode database (Hermes' own kanban.db among them) defeat a plain +# extension check. Match on the basename so both the main file and the +# sidecars are covered. +_SQLITE_DB_FAMILY_RE = re.compile( + r"\.(db|sqlite|sqlite3)([-.](wal|shm|journal))?$", re.IGNORECASE +) + + 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 formats are always rejected; @@ -434,6 +449,14 @@ def _check_binary_document_write(filepath: str, task_id: str = "default") -> str "bytes). Use the docx/xlsx/powerpoint skills or a library like " "python-docx/openpyxl/python-pptx via the terminal to create or edit " "this document.") + if _SQLITE_DB_FAMILY_RE.search(os.path.basename(filepath or "")): + return ( + f"Refusing to write plain text to database file '{filepath}'. " + "A text write can never produce a valid database, and the text tools' " + "read->modify->write round-trip re-encodes the binary bytes lossily " + "(the terminal transport decodes stdout with errors=replace), silently " + "destroying the file. Use the sqlite3 CLI or a SQLite library via the " + "terminal for any database change.") if is_pdf_path(filepath): try: resolved = Path(_resolve_path_for_task(filepath, task_id)) @@ -449,6 +472,28 @@ def _check_binary_document_write(filepath: str, task_id: str = "default") -> str "is allowed.)") except OSError: pass + return None + # Other binary extensions (images, archives, fonts, executables, ...): + # same corruption argument as the PDF-overwrite rule — the model can only + # have "read" an existing binary as mojibake, so writing text back destroys + # it. Creating a NEW file with a binary extension stays allowed (a text + # file with an unlucky extension like .dat is odd but harmless). + if 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(): + ext = os.path.splitext(filepath)[1].lower() + 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. " + "(Creating a NEW file with this extension is allowed.)") + except OSError: + pass return None