Fix memory-safety bugs in the C helper layer

[carpentry-agent]

Fixes three memory-safety bugs in sqlite3_helper.h, all in the result/error handling around query, exec-prepared, prepare, and open.

1. Use-after-free of the error message

sqlite3_errmsg returns memory that SQLite invalidates on the next library call. Both SQLite3_exec_c and SQLite3_exec_prepared_c captured that pointer and then called sqlite3_finalize / sqlite3_reset before stashing it in the result, so the error string handed back to Carp was read after it had been invalidated. The message is now copied into owned memory before the statement is torn down (SQLite3_copy_errmsg), matching what open/errmsg already did. SQLiteRes.error now returns an owned String instead of a borrowed (Ptr CChar).

2. Text/blob parameter buffers leaked on every call

SQLiteColumn had no delete, so the buffers backing text/blob query parameters (handed to C from Carp Strings/Arrays) were never freed — a leak on every query/exec-prepared that binds a string or blob. SQLiteColumn now implements delete (and copy); dropping the parameter array frees those buffers. Result columns are unaffected because their buffers are moved into Carp values before the column is dropped.

3. Row container arrays leaked after to-array

SQLiteRes had no delete, so the per-row column arrays (row->data) and the row array (rows.rows) were never freed once to-array had consumed the result. SQLiteRes now implements delete, freeing those containers (the column buffers themselves belong to Carp by then, so they are left intact).

The error paths now also free partially-built result rows, which the union-overwrite with the error string previously dropped.

Verification

• All existing tests pass; added two assertions that the error message survives statement teardown (one for the query/finalize path, one for the exec-prepared/reset path).
• Added test/memory.carp, a --log-memory regression test that runs 50 mixed query/prepared/error workloads (text + blob params, result sets, syntax and constraint errors) and asserts the allocation balance is unchanged. It fails if any of the three fixes is reverted. Run with carp -x --log-memory test/memory.carp.

Note: I'd have added a CI step for the memory test, but the heartbeat app token lacks workflows permission, so .github/workflows/ci.yml is untouched — adding a carp -x --log-memory test/memory.carp step is a one-liner for a maintainer.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

[carpentry-agent]

The only thing blocking CI was an angler unsafe-result-unwrap finding on test/memory.carp:10 — the in-memory open was unwrapped with Result.unsafe-from-success. Switched it to a match (the success path is identical, so the memory-balance assertion is unchanged — still passes with no net allocations across 50 workloads). The C helper-layer fixes are untouched. CI is now green on both platforms, so I've marked this ready for review.

[carpentry-reviewer]

Build & Tests

Checked out claude/fix-memory-safety, built against system libsqlite3, and ran everything locally:

• carp -x test/sqlite3.carp → 36 passed, 0 failed, including the two new error-survival assertions and the existing blob with embedded nulls round-trips test.
• carp -x --log-memory test/memory.carp → net-zero allocations across 50 workloads (passes).
• carp -x gendocs.carp → clean.
• CI green on ubuntu-latest and macos-latest.

To confirm the memory test actually has teeth, I temporarily neutered SQLite3_SQLiteColumn_delete to a no-op and re-ran it: it failed loudly with Expected '0', actual '4000' (≈80 leaked param buffers/workload × 50), then I restored the file. So the regression guard genuinely catches fix #2 being reverted.

Findings

I traced the full ownership model end to end and tried to find a leak, double-free, or use-after-free in each of the three fixes:

• Fix #1 (errmsg UAF): in both SQLite3_exec_c (sqlite3_helper.h:351) and SQLite3_exec_prepared_c (sqlite3_helper.h:411), SQLite3_copy_errmsg(err) runs before sqlite3_finalize/sqlite3_reset, so the message is copied while still valid. The two new tests exercise both paths and pass. ✓
• Fix #2 (param buffer leak): to-sqlite3-internal moves the Carp String/Array buffer into the column; the temp (Array SQLiteColumn) from copy-map is dropped after exec-, and the new delete frees each text/blob buffer exactly once. Binds use SQLITE_STATIC, which is safe because that array outlives the synchronous exec. The tag guard in delete/copy correctly prevents touching the union for int/float/null columns. ✓
• Fix #3 (container leak): result column buffers are moved into Carp values — from-text hands col.s back as a String, from-blob as an Array{data=col.s} — so free_containers correctly frees only the row/data arrays, not the column buffers. The OK result is consumed by to-array, whose delete runs free_containers once. ✓

Other things I checked and confirmed safe:

• res.rows is initialized (sqlite3_helper.h:328/399) before any goto, so free_all on an empty/partial result is well-defined (free(NULL) safe).
• The union is written (res.err) only after free_all reads res.rows — correct ordering, no clobber.
• Every SQLiteRes is consumed by to-array (OK) or error (ERR) in query/exec-prepared, so an ERR result's err buffer is always transferred to a String and never hits the is == OK no-op in delete.

Two minor, non-blocking notes for the future (neither affects current behavior):

1. SQLite3_SQLiteColumn_copy sizes TEXT via strlen+1, so it would truncate a text value containing an embedded NUL. copy isn't exercised on the hot path today (params are moved, results are moved), and text params come from Carp Strings, so this is purely theoretical — but if a column array is ever @-copied with NUL-bearing text it'd be lossy.
2. SQLiteRes.delete no-ops on ERR, relying on the invariant that ERR results are always consumed by error. That holds throughout the current code; just worth keeping in mind if a new code path ever drops a result directly.

Prior activity: the only prior commit beyond the fix is the heartbeat's own follow-up swapping Result.unsafe-from-success for a match in the memory test to clear an angler finding — verified that's behavior-preserving (the memory-balance assertion still holds).

Verdict: merge

All three fixes are correct and verified end to end; the memory regression test is sensitive and passes at net-zero; functional suite and docs are green on both platforms. The two notes above are future-proofing, not blockers.