fix(tools): refuse text writes into database files and binary overwrites

[garadice]

Problem

write_file and patch accept database and other binary files as text targets. The tools read such files as lossy text (errors=replace on stdout decode) and write the mojibake back — silently destroying the file. This is not hypothetical: it destroyed this fleet's live kanban.db (550k bytes → U+FFFD, unrecoverable; SQLite database disk image is malformed).

The corruption path is structural: a text tool can never round-trip binary content, so accepting the target at all is the defect. Four sibling read paths already sniff binaries; the write paths did not.

Fix

Extends the existing _check_binary_document_write guard in tools/file_tools_write_guards.py (the established write-guard home, following the repo's own PDF precedent):

• Database family, always refused (main file AND -wal/-shm/-journal sidecars — os.path.splitext("x.db-wal") yields .db-wal, which is not in BINARY_EXTENSIONS, so a plain extension check misses every WAL sidecar; matched via basename regex). Error message names the right tool (sqlite3 CLI / SQLite library).
• Other binary extensions refused on OVERWRITE (same corruption argument as the existing PDF-overwrite rule: the model can only have "read" an existing binary as mojibake). Creating a NEW file with a binary extension stays allowed — a text file with an unlucky extension is odd but harmless.

Tests

tests/tools/test_binary_db_write_guard.py — 29/29 new tests (db main + sidecars, overwrite-refused/new-file-allowed matrix, NUL-content refusal, text files unaffected). Full guard + patch + file-tools suites green (196 tests); the one failing test in the tree (test_approval smart-approval) fails identically on clean origin/main (verified via stash).

Verified end-to-end against a live SQLite database: refusal fires before any byte is read or written; the DB stays byte-identical with integrity_check ok.

[alt-glitch]

This was generated by AI during triage.

Related: merged #86362 introduced binary-document write refusal; this extends the same guard to SQLite files and sidecars.

[strzhao]

Ran the PR head (5c22d32139) against a clean origin/main (98f758ae7e) checkout, Python 3.11.16 / SQLite 3.53.1, driving the real tool entry points instead of reading the diff.

Reproduced on main first. With a WAL-mode database whose sidecars exist:

• read_file("<dir>/state.db-wal") succeeds and returns 185 characters of lossy mojibake.
• write_file("<dir>/state.db-wal", "PLAIN TEXT OVERWRITE") succeeds, 20 bytes written.
• The next sqlite3.connect("<dir>/state.db") raises file is not a database.

-shm behaves the same. On the PR head both writes are refused by the new rule, the bytes are unchanged and the database still opens, so the destructive path is closed end to end, and your suite backs it: 12 passed on the head, failing on base.

Two cases the name-based rule does not reach, both reproduced on the PR head:

1. A database whose main file has no extension stays fully writable. <dir>/holder is a real SQLite database held open by a live WAL connection; read_file hands back mojibake and write_file("<dir>/holder", "PLAIN TEXT OVERWRITE") lands, after which the database is file is not a database. holder-wal behaves the same. The regex anchors on .db|.sqlite|.sqlite3 in the basename, so an extensionless DB triple misses both the new branch and has_binary_extension. Extensionless triples do occur in this tree: the state.db holder tests leave exactly holder, holder-wal, holder-shm on disk.
2. The read half is still open on both revisions. read_file("<dir>/state.db-wal") returns lossy text on main and on this head, because _has_extension_in compares the last dot-suffix (.db-wal) against BINARY_EXTENSIONS and misses. Your guard's own message names the consequence ("read_file showed you ... mojibake text"); that mojibake is where the round trip starts.

Nothing needed for the merge decision: the write-side fix stands on its own, and the sidecars are the live hole, since .db on main is already refused twice (the read guard, then the no-baseline branch of the stale-overwrite guard).

For (2) I have a one-function change to tools/binary_extensions.py that teaches the shared helper to see DB sidecars, so the read guard refuses and this write guard agrees with it. Cherry-picking that commit into this PR is fine if you would rather keep the fix in one place; otherwise I will send it standalone.

[strzhao]

Follow-up on case (2) from my review above — the standalone PR is up: #114159

It is one function in tools/binary_extensions.py (has_binary_extension also matches a .db / .sqlite / .sqlite3 name carrying -wal, -shm or -journal) plus a 3-case test: 3 passed on that head, 2 of them failing on 98f758ae7e.

First preference: lift it as one small commit keeping authorship — git cherry-pick b0401fe489 onto this branch, and the read guard refuses the same sidecars your write guard already blocks. If you would rather write it yourself, a Co-authored-by on the absorbing commit works. If neither fits, the standalone PR stands on its own and nothing here needs to change.

[kshitijk4poor]

Landed on main via #114220 (merge 6e7dad7608), cherry-picked with your authorship preserved — thank you, @garadice.

The salvage PR body records what was kept, what was trimmed and why; the merged tests are the invariant subset. If anything in the landed shape misses your intent, comment there and I will follow up.