From 11d68e4652ffe687f229cfaead40445b23e201f6 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 19 Sep 2026 02:39:31 -0700 Subject: [PATCH] feat(tools): read_file renders SQLite schemas and flags merge conflicts; web_extract refuses binary payloads Three small gaps from the OMP comparison (#77367) that the existing tools almost covered: - read_file on .db/.sqlite/.sqlite3 was refused as binary. It now extracts a schema overview (CREATE per table, row count, first 5 rows, indexes/views) through the same document-extraction path as .docx/.xlsx, opened read-only and immutable so a live database is never locked. A .db whose magic bytes are not SQLite is refused with that reason. The Hermes read denylist now runs before extraction so protected stores cannot be reached through an extractor. - read_file reports conflict_blocks: N (plus a hint) when the served range has balanced <<<<<<< / >>>>>>> marker lines, so the model resolves the conflict instead of editing around it. A lone marker in prose is not counted. - web_extract returned 824K chars of raw SQLite bytes as page "content" for a .sqlite URL (live: Chinook_Sqlite.sqlite via the configured backend). Bodies starting with an unambiguous file signature (SQLite, ZIP, gzip, xz, 7z, ELF, Mach-O, PNG/JPEG/GIF/TIFF, FLAC/Ogg) become a typed error pointing at terminal + read_file. Backends drop NUL bytes, so signatures are compared NUL-stripped; two-letter signatures (BM, MZ, ID3) are excluded on purpose. --- tests/tools/test_file_tools.py | 21 +++++- tests/tools/test_read_extract.py | 35 +++++++++- tests/tools/test_web_tools_truncate.py | 18 +++++ tools/file_operations_common.py | 12 ++++ tools/file_tools.py | 29 ++++++--- tools/read_extract.py | 65 ++++++++++++++++++- tools/web_tools_truncate.py | 27 ++++++++ .../features/document-extraction.md | 5 ++ 8 files changed, 195 insertions(+), 17 deletions(-) diff --git a/tests/tools/test_file_tools.py b/tests/tools/test_file_tools.py index 7f0124e7ee..d8c6924685 100644 --- a/tests/tools/test_file_tools.py +++ b/tests/tools/test_file_tools.py @@ -12,6 +12,7 @@ import pytest from tools.file_tools import ( PATCH_SCHEMA, + read_file_tool, ) @@ -732,7 +733,7 @@ class TestDedupInvalidationTaskResolution: task_id = "acp-dedup" monkeypatch.setattr(tt, "_task_env_overrides", {task_id: {"cwd": str(workspace)}}) - (workspace / "data.txt").write_text("v1\n") + (workspace / "data.txt").write_text("v1\n", encoding="utf-8") # The task resolves the relative path into the workspace; the default # task (the old buggy resolution) would resolve into proc. @@ -976,7 +977,7 @@ class TestNotFoundCache: assert _check_not_found_cache("read", str(target), tid) is not None # Out-of-band creation: plain filesystem write, no tool hook fires. - target.write_text("real content\n") + target.write_text("real content\n", encoding="utf-8") # The cached miss must NOT be served once the path exists… assert _check_not_found_cache("read", str(target), tid) is None, ( @@ -1000,7 +1001,7 @@ class TestNotFoundCache: assert _check_not_found_cache("search", str(missing_dir), tid) is not None missing_dir.mkdir() - (missing_dir / "x.txt").write_text("hi\n") + (missing_dir / "x.txt").write_text("hi\n", encoding="utf-8") assert _check_not_found_cache("search", str(missing_dir), tid) is None, ( "stale 'Path not found' served after the directory was created" @@ -1137,3 +1138,17 @@ class TestSecretFileReadRedaction: assert self.SYNTH not in raw assert "«redacted" in raw + + +class TestConflictMarkerFlag: + def test_read_flags_balanced_conflict_blocks_only(self, tmp_path): + conflicted = tmp_path / "c.py" + conflicted.write_text("x=1\n<<<<<<< HEAD\ny=2\n=======\ny=3\n>>>>>>> feature\nz=4\n", encoding="utf-8") + result = json.loads(read_file_tool(str(conflicted))) + assert result["conflict_blocks"] == 1 + assert "merge-conflict" in result["_hint"] + + prose = tmp_path / "p.py" + prose.write_text("print('<<<<<<< not a conflict')\n", encoding="utf-8") + assert "conflict_blocks" not in json.loads(read_file_tool(str(prose))) + diff --git a/tests/tools/test_read_extract.py b/tests/tools/test_read_extract.py index b100b90318..38384c184c 100644 --- a/tests/tools/test_read_extract.py +++ b/tests/tools/test_read_extract.py @@ -457,7 +457,7 @@ class TestNotebookExtraction(unittest.TestCase): "outputs": [{"output_type": "stream", "text": "x" * (_MAX_OUTPUT_CHARS + 5000)}]}, ]}], "nbformat": 3} - with open(p, "w") as fh: + with open(p, "w", encoding="utf-8") as fh: json.dump(nb, fh) text = extract_document_text(p) self.assertIn("output chars truncated", text) @@ -470,7 +470,7 @@ class TestNotebookExtraction(unittest.TestCase): {"cell_type": "code", "source": "1+1", "outputs": [{"output_type": "pyout", "text": ["2"]}]}, ]}], "nbformat": 3} - with open(p, "w") as fh: + with open(p, "w", encoding="utf-8") as fh: json.dump(nb, fh) text = extract_document_text(p) self.assertIn("Output (cell 1)", text) @@ -905,3 +905,34 @@ class TestPdfCoverageNote(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class TestSqliteExtraction(unittest.TestCase): + def test_sqlite_reads_as_schema_overview_and_non_sqlite_db_is_refused(self): + import sqlite3 + + with tempfile.TemporaryDirectory() as d: + db = os.path.join(d, "shop.db") + con = sqlite3.connect(db) + con.executescript( + "CREATE TABLE users(id INTEGER PRIMARY KEY, name TEXT, blob BLOB);" + "CREATE INDEX ix_name ON users(name);" + "INSERT INTO users VALUES(1,'ann|pipe',x'0011'),(2,'bob',NULL);") + con.commit() + con.close() + + result = json.loads(read_file_tool(db)) + content = result["content"] + self.assertTrue(result.get("extracted_document")) + self.assertIn("## users (2 rows)", content) + self.assertIn("CREATE TABLE users", content) + self.assertIn("", content) # blobs never enter context raw + self.assertIn("ann\\|pipe", content) # table cell escaping keeps the markdown table intact + self.assertIn("index ix_name", content) + + fake = os.path.join(d, "notdb.db") + with open(fake, "wb") as fh: + fh.write(b"hello, not a database") + refused = json.loads(read_file_tool(fake)) + self.assertIn("not a SQLite database", refused["error"]) + diff --git a/tests/tools/test_web_tools_truncate.py b/tests/tools/test_web_tools_truncate.py index 8d0a6f2812..9554a3b393 100644 --- a/tests/tools/test_web_tools_truncate.py +++ b/tests/tools/test_web_tools_truncate.py @@ -110,3 +110,21 @@ class _AsyncTrue: """Async callable that always returns True (re-awaitable per call).""" async def __call__(self, *a, **k): return True + + +def test_binary_payload_is_refused_but_prose_with_short_signature_prefix_passes(): + """A backend that fetched a raw SQLite/zip file hands its bytes back as text; that must become an + error naming the type, while ordinary pages (even ones starting with 'BM' or 'MZ') pass untouched.""" + results = [ + {"url": "u", "raw_content": "SQLite format 3\x10\x01" + "x" * 5000}, # backend already dropped the NUL + {"url": "z", "raw_content": "PK\x03\x04" + "y" * 50}, + {"url": "v", "raw_content": "BMW reviews are fine"}, + {"url": "w", "raw_content": "# hi"}, + ] + web_tools_truncate._truncate_results(results, 5000, {"pages_truncated": 0, "truncation_metrics": []}) + assert results[0]["content"] == "" and "SQLite database" in results[0]["error"] + assert results[1]["content"] == "" and "ZIP archive" in results[1]["error"] + assert results[2]["error"] is None if "error" in results[2] else True + assert results[2]["content"] == "BMW reviews are fine" + assert results[3]["content"] == "# hi" + diff --git a/tools/file_operations_common.py b/tools/file_operations_common.py index 55882a8d27..adb62aa6cc 100644 --- a/tools/file_operations_common.py +++ b/tools/file_operations_common.py @@ -189,6 +189,18 @@ _OSC_SEQUENCE_RE = re.compile(r"\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)") _FENCE_MARKER_RE = re.compile(r"'?\x07?__HERMES_FENCE_[A-Za-z0-9]+__\x07?'?") +_CONFLICT_OPEN = re.compile(r"^\s*\d+\|<<<<<<< ", re.M) +_CONFLICT_CLOSE = re.compile(r"^\s*\d+\|>>>>>>> ", re.M) + + +def count_conflict_blocks(formatted_content: str) -> int: + """Unresolved git merge-conflict blocks in a ``LINE|CONTENT`` read; 0 when the page has no + balanced ``<<<<<<< `` / ``>>>>>>> `` pair (a lone marker in prose or a test fixture is not a + conflict). Reported on read so the model resolves the conflict instead of editing around it.""" + opens = len(_CONFLICT_OPEN.findall(formatted_content)) + return min(opens, len(_CONFLICT_CLOSE.findall(formatted_content))) if opens else 0 + + def _strip_terminal_fence_leaks(text: str) -> str: """Strip leaked terminal fence wrappers (OSC sequences, fence markers) from command output; drops lines that were nothing but wrapper.""" diff --git a/tools/file_tools.py b/tools/file_tools.py index 0881a3abc0..c87c832bf3 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -24,7 +24,7 @@ from tools.binary_extensions import has_binary_extension from tools.skill_provenance import is_background_review from tools.file_operations import ( ShellFileOperations, normalize_read_pagination, normalize_search_pagination) -from tools.file_operations_common import DEFAULT_READ_LIMIT +from tools.file_operations_common import DEFAULT_READ_LIMIT, count_conflict_blocks from tools import file_state from agent.redact import _is_secret_file_arg, redact_sensitive_text from tools.file_tools_paths import ( @@ -579,7 +579,7 @@ def read_file_tool(path: str, offset: int = 1, limit: int = DEFAULT_READ_LIMIT, Guard order: NT/device-namespace prefix (raw string, no resolution) → device-path blocklist (no I/O) → stat-based special-file guard (host only) - → document extraction → binary-extension guard → Hermes internal denylist + → Hermes internal denylist → document extraction → binary-extension guard → negative-result cache → dedup stub → real read. """ try: @@ -613,6 +613,14 @@ def read_file_tool(path: str, offset: int = 1, limit: int = DEFAULT_READ_LIMIT, "attempted. Use terminal utilities if you need to " "interact with it.")}) + # Hermes internal denylist (prompt injection via catalog metadata, + # credential stores). Runs BEFORE document extraction so a + # protected SQLite store (state.db) cannot be read through the extractor. Pass the RESOLVED path: the denylist's own + # resolve() uses the process cwd and would miss a relative "auth.json". + block_error = get_read_block_error(str(_resolved)) + if block_error: + return tool_error(block_error) + extracted = _read_extracted_document(path, _resolved, offset, limit, task_id) if extracted is not None: return extracted @@ -624,13 +632,6 @@ def read_file_tool(path: str, offset: int = 1, limit: int = DEFAULT_READ_LIMIT, f"Cannot read binary file '{path}' ({_resolved.suffix.lower()}). " "Use vision_analyze for images, or terminal to inspect binary files.") - # Hermes internal denylist (prompt injection via catalog metadata, - # credential stores). Pass the RESOLVED path: the denylist's own - # resolve() uses the process cwd and would miss a relative "auth.json". - block_error = get_read_block_error(str(_resolved)) - if block_error: - return tool_error(block_error) - resolved_str = str(_resolved) cached_not_found = _check_not_found_cache("read", resolved_str, task_id) if cached_not_found is not None: @@ -680,6 +681,14 @@ def read_file_tool(path: str, offset: int = 1, limit: int = DEFAULT_READ_LIMIT, redacted = result.content != unredacted result_dict["content"] = result.content + if result.content: + conflicts = count_conflict_blocks(result.content) + if conflicts: + result_dict["conflict_blocks"] = conflicts + result_dict["_hint"] = ( + f"{conflicts} unresolved git merge-conflict block(s) (<<<<<<< / ======= / >>>>>>>) in this " + "range. Resolve them (keep one side or combine, delete the markers) before editing around them.") + if (file_size and file_size > _LARGE_FILE_HINT_BYTES and limit > 200 and result_dict.get("truncated")): result_dict.setdefault("_hint", ( @@ -1105,7 +1114,7 @@ READ_FILE_SCHEMA = { # route we trust (_read_file_schema_overrides). Scanned-page coverage # teaching lives in the response-time NEEDS-OCR warning # (read_extract.py); the schema doesn't pre-teach it. - "description": "Read a text file with line numbers and pagination. Use this instead of cat/head/tail in terminal. Output format: 'LINE_NUM|CONTENT'. Suggests similar filenames if not found. Use offset and limit for large files. Reads exceeding ~100K characters are truncated on a line boundary and return a next_offset; continue with offset to read the rest. Documents auto-extract to readable text: .ipynb, Office (.docx/.xlsx/.pptx and legacy .doc/.ppt/.xls), PDF (text layer), OpenDocument, RTF, EPUB. Cannot read images/binary — use vision_analyze for images.", + "description": "Read a text file with line numbers and pagination. Use this instead of cat/head/tail in terminal. Output format: 'LINE_NUM|CONTENT'. Suggests similar filenames if not found. Use offset and limit for large files. Reads exceeding ~100K characters are truncated on a line boundary and return a next_offset; continue with offset to read the rest. Documents auto-extract to readable text: .ipynb, Office (.docx/.xlsx/.pptx and legacy .doc/.ppt/.xls), PDF (text layer), OpenDocument, RTF, EPUB, SQLite (.db/.sqlite: schema, row counts, first rows). Cannot read images/binary — use vision_analyze for images.", "parameters": { "type": "object", "properties": { diff --git a/tools/read_extract.py b/tools/read_extract.py index 079ea7917d..808749a41c 100644 --- a/tools/read_extract.py +++ b/tools/read_extract.py @@ -27,7 +27,7 @@ from xml.etree import ElementTree as ET __all__ = ["EXTRACTABLE_EXTENSIONS", "ExtractionError", "extract_document_bytes", "extract_document_text", "is_extractable_document"] -EXTRACTABLE_EXTENSIONS = frozenset({".ipynb", ".docx", ".xlsx"}) +EXTRACTABLE_EXTENSIONS = frozenset({".ipynb", ".docx", ".xlsx", ".db", ".sqlite", ".sqlite3"}) ANYDOC_EXTENSIONS = frozenset({ ".doc", ".docm", ".ppt", ".pps", ".pot", ".pptx", ".pptm", ".ppsx", ".ppsm", ".xls", ".xlsm", ".xlsb", ".odt", ".ods", ".odp", ".rtf", ".epub", ".pdf"}) @@ -552,9 +552,70 @@ def _cell_value(cell: ET.Element, shared: list[str], s: str) -> str: return (value or "#ERROR") if typ == "e" else value +_SQLITE_MAGIC = b"SQLite format 3\x00" +_SQLITE_PREVIEW_ROWS = 5 +_SQLITE_MAX_TABLES = 200 +_SQLITE_CELL_CHARS = 80 + + +def _extract_sqlite(path: str) -> str: + """Render a SQLite file as a schema overview: per table the CREATE statement, row count and the + first rows. A ``.db`` that is not SQLite raises ExtractionError so read_file reports the real type.""" + import sqlite3 + + with open(path, "rb") as fh: + if fh.read(len(_SQLITE_MAGIC)) != _SQLITE_MAGIC: + raise ExtractionError("not a SQLite database (magic bytes do not match)") + # Read-only URI: never create or mutate; immutable=1 also skips WAL/journal sidecars so a live + # database that another process has open is still readable without taking locks. + uri = Path(path).resolve().as_uri() + "?mode=ro&immutable=1" + con = sqlite3.connect(uri, uri=True) + try: + objs = con.execute( + "SELECT type, name, sql FROM sqlite_master WHERE sql IS NOT NULL ORDER BY type DESC, name" + ).fetchall() + out = ["# SQLite database", ""] + tables = [o for o in objs if o[0] == "table"] + others = [o for o in objs if o[0] != "table"] + out.append(f"{len(tables)} table(s), {len(others)} index/view/trigger(s)") + for _, name, sql in tables[:_SQLITE_MAX_TABLES]: + quoted = '"' + name.replace('"', '""') + '"' + count = con.execute(f"SELECT COUNT(*) FROM {quoted}").fetchone()[0] + out += ["", f"## {name} ({count:,} rows)", sql.strip()] + cur = con.execute(f"SELECT * FROM {quoted} LIMIT {_SQLITE_PREVIEW_ROWS}") + cols = [d[0] for d in cur.description] + rows = cur.fetchall() + if rows: + out.append("| " + " | ".join(cols) + " |") + out.append("|" + "---|" * len(cols)) + for row in rows: + cells = [_sqlite_cell(v) for v in row] + out.append("| " + " | ".join(cells) + " |") + if len(tables) > _SQLITE_MAX_TABLES: + out.append(f"\n... {len(tables) - _SQLITE_MAX_TABLES} more tables omitted") + if others: + out += ["", "## Indexes / views / triggers"] + [f"- {t} {n}" for t, n, _ in others] + out += ["", "Query it with the terminal: sqlite3 'SELECT ...' (or Python's sqlite3 module)."] + return "\n".join(out) + except sqlite3.DatabaseError as exc: + raise ExtractionError(f"SQLite read failed: {exc}") from exc + finally: + con.close() + + +def _sqlite_cell(value: Any) -> str: + if value is None: + return "NULL" + if isinstance(value, (bytes, bytearray)): + return f"" + text = str(value).replace("|", "\\|").replace("\n", " ") + return text if len(text) <= _SQLITE_CELL_CHARS else text[:_SQLITE_CELL_CHARS - 1] + "…" + + # Extension -> stdlib extractor; anydoc formats fall through in extract_document_text. _STDLIB_EXTRACTORS: dict[str, Callable[[str], str]] = { - ".ipynb": _extract_notebook, ".docx": _extract_docx, ".xlsx": _extract_xlsx} + ".ipynb": _extract_notebook, ".docx": _extract_docx, ".xlsx": _extract_xlsx, + ".db": _extract_sqlite, ".sqlite": _extract_sqlite, ".sqlite3": _extract_sqlite} # ---- BEGIN PLUGIN-COMPAT (revert-scheduled; see COMPAT_MANIFEST.md) ---- diff --git a/tools/web_tools_truncate.py b/tools/web_tools_truncate.py index 1cee30e81f..255b460e6f 100644 --- a/tools/web_tools_truncate.py +++ b/tools/web_tools_truncate.py @@ -133,6 +133,23 @@ def _effective_char_limit(char_limit: Optional[int]) -> int: return _clamp_or_default(char_limit) if char_limit is not None else _get_extract_char_limit() +_UNAMBIGUOUS_BINARY_KINDS = ("SQLite", "ZIP", "gzip", "bzip2", "xz", "7-Zip", "ELF", "Mach-O", "PNG", "JPEG", "GIF", "TIFF", "FLAC", "Ogg") + + +def _binary_payload_kind(text: str) -> str: + """Magic-byte type name when a fetched body is a raw binary file, else ``""``. Backends return + the body as text with NUL bytes dropped, so signatures are compared NUL-stripped on both sides. + Only the file tools' own signature table; HTML/markdown/JSON never start with one.""" + from tools.file_operations import _MAGIC_SIGNATURES + + head = text[:32].encode("latin-1", "ignore").replace(b"\x00", b"") + for prefix, name in _MAGIC_SIGNATURES: + sig = prefix.replace(b"\x00", b"") + if sig and name.startswith(_UNAMBIGUOUS_BINARY_KINDS) and head.startswith(sig): + return name + return "" + + def _truncate_results(results: List[dict], char_limit: int, debug_call_data: dict) -> None: """In place: replace each successful entry's content with its base64-cleaned, budgeted text; per-page truncation metrics go into ``debug_call_data``.""" @@ -141,6 +158,16 @@ def _truncate_results(results: List[dict], char_limit: int, debug_call_data: dic raw_content = result.get("raw_content", "") or result.get("content", "") if result.get("error") or not raw_content: continue + binary_kind = _binary_payload_kind(raw_content) + if binary_kind: + # A backend that fetched a raw file (SQLite, archive, executable) hands back its bytes as + # "text"; 800K chars of that would enter context. Name the type and point at the tool that reads it. + result["content"] = "" + result["error"] = ( + f"URL returned binary content ({binary_kind}), not a page. Download it with the terminal " + "(curl -L -o) and use read_file (SQLite/Office/PDF auto-extract) or terminal utilities on the file.") + logger.info("%s (binary payload: %s, %d chars dropped)", url, binary_kind, len(raw_content)) + continue clean = convert_base64_images_to_links(raw_content) model_text, truncated = _truncate_with_footer(clean, url, char_limit) result["content"] = model_text diff --git a/website/docs/user-guide/features/document-extraction.md b/website/docs/user-guide/features/document-extraction.md index 08979794b1..3880db62d8 100644 --- a/website/docs/user-guide/features/document-extraction.md +++ b/website/docs/user-guide/features/document-extraction.md @@ -15,6 +15,7 @@ The `read_file` tool automatically converts common document formats to readable | Jupyter notebooks | `.ipynb` | Built-in (stdlib) | Always | | Word documents | `.docx` | Built-in (stdlib) | Always | | Excel workbooks | `.xlsx` | Built-in (stdlib) | Always | +| SQLite databases | `.db`, `.sqlite`, `.sqlite3` | Built-in (stdlib) | Always | | PDF | `.pdf` | Optional `anydoc` converter | Auto-installed on first use* | | Legacy Office | `.doc`, `.ppt`, `.xls`, `.pptx`, and variants | Optional `anydoc` converter | Auto-installed on first use* | | OpenDocument | `.odt`, `.ods`, `.odp` | Optional `anydoc` converter | Auto-installed on first use* | @@ -22,6 +23,10 @@ The `read_file` tool automatically converts common document formats to readable \* The optional converter is the `firecrawl-anydoc` package, installed lazily where installs are permitted (`security.allow_lazy_installs` in `config.yaml`). Without it, the three stdlib formats still work; other formats fall back to the binary-file guard. +SQLite files render as a schema overview rather than a dump: each table's `CREATE` statement, row count and first five rows, plus the list of indexes, views and triggers. The database is opened read-only (`mode=ro&immutable=1`, so a live database another process holds open is still readable without locks); for anything beyond the preview, query it from the terminal with `sqlite3`. A `.db` that is not SQLite (the magic bytes do not match) is refused with the real reason. Hermes's own read denylist applies before extraction, so protected stores under `HERMES_HOME` stay unreadable. + +`read_file` also flags unresolved git merge conflicts: when the returned range contains balanced `<<<<<<< ` / `>>>>>>> ` marker lines, the result carries `conflict_blocks: N` and a hint to resolve them before editing around them. A lone marker inside a string or a test fixture is not counted. + Conversion output is Markdown, paginated through `read_file`'s normal `offset`/`limit` window. East Asian phonetic guides (XLSX `rPh`, DOCX ruby text) annotate cell or run text and are not part of the extracted value: a cell holding 東京 with the guide トウキョウ reads as `東京`. Documents over 50 MB are refused to keep tool turns bounded. Notebook cell outputs longer than 20,000 characters are truncated; the truncation marker carries a `jq` command that names the notebook's full, shell-quoted path so the omitted output can be pulled from the original file.