VACUUM snapshot metadata through the sqlite driver, not a CLI (closes #120) #136

Merged
clawbot merged 1 commits from issue-120-vacuum-via-driver into next 2026-09-21 19:41:29 +02:00
Collaborator

Fixes #120.

snapshot create vacuumed the per-snapshot metadata database by running an external sqlite3 binary, so a backup failed at the very end — after all the expensive upload work — on any host without that CLI on PATH. The suite hid this because bootstrap installed the CLI.

vacuumDatabase now opens the temporary database with the modernc.org/sqlite driver and runs VACUUM through it, outside any transaction; internal/snapshot no longer imports os/exec. The security-critical VACUUM — dropping pages that held deleted rows before the file is uploaded — is unchanged in intent.

What a reader should know: the database opens in WAL mode, so VACUUM's rewrite lands in the WAL. The close right after VACUUM checkpoints it into the main snapshot.db file, which is the file compressFile reads for upload, so the uploaded file is the compacted one. A new test inserts rows carrying a marker, deletes them, vacuums, and asserts the file both shrank and no longer contains the marker bytes.

script/bootstrap and the Dockerfile test image no longer install the CLI; the stale Dockerfile comment (which named a CGO driver this repo does not use) is corrected.

make check is green run with no sqlite3 on PATH (command -v sqlite3 printed nothing; the one directory that held it was removed from PATH).

Disclosures:

  • The Dockerfile test-image change was not exercised by local make check, which builds only the lint image.
  • Removed a now-false sentence from the dated 2026-08-07 TODO.md entry so the sqlite3 grep is clean.

Model: opus-4-8

Fixes https://git.eeqj.de/sneak/vaultik/issues/120. `snapshot create` vacuumed the per-snapshot metadata database by running an external `sqlite3` binary, so a backup failed at the very end — after all the expensive upload work — on any host without that CLI on `PATH`. The suite hid this because bootstrap installed the CLI. `vacuumDatabase` now opens the temporary database with the `modernc.org/sqlite` driver and runs `VACUUM` through it, outside any transaction; `internal/snapshot` no longer imports `os/exec`. The security-critical VACUUM — dropping pages that held deleted rows before the file is uploaded — is unchanged in intent. What a reader should know: the database opens in WAL mode, so VACUUM's rewrite lands in the WAL. The close right after VACUUM checkpoints it into the main `snapshot.db` file, which is the file `compressFile` reads for upload, so the uploaded file is the compacted one. A new test inserts rows carrying a marker, deletes them, vacuums, and asserts the file both shrank and no longer contains the marker bytes. `script/bootstrap` and the `Dockerfile` test image no longer install the CLI; the stale `Dockerfile` comment (which named a CGO driver this repo does not use) is corrected. `make check` is green run with no `sqlite3` on `PATH` (`command -v sqlite3` printed nothing; the one directory that held it was removed from `PATH`). Disclosures: - The `Dockerfile` test-image change was not exercised by local `make check`, which builds only the lint image. - Removed a now-false sentence from the dated 2026-08-07 `TODO.md` entry so the sqlite3 grep is clean. Model: opus-4-8
clawbot added the needs-review label 2026-09-21 15:11:57 +02:00
clawbot self-assigned this 2026-09-21 15:11:57 +02:00
clawbot added needs-rework and removed needs-review labels 2026-09-21 19:05:27 +02:00
Author
Collaborator

FAIL

The VACUUM-through-driver change and its new test are correct, but the production runtime image still ships the exact dependency this change exists to remove.

  • Dockerfile:76 (runtime stage) still runs apk add --no-cache ca-certificates sqlite. The sqlite package provides the sqlite3 CLI that the old snapshot create shelled out to at runtime; now that the shell-out is gone, nothing in the pure-Go binary invokes it. This contradicts the comment this PR adds to the builder stage ("no sqlite library or CLI is required") and leaves the shipped image carrying the dependency that #120 sets out to drop. Acceptable: remove sqlite from the runtime apk add (keep ca-certificates), or state in the PR why it must stay.

Model: opus-4-8

FAIL The VACUUM-through-driver change and its new test are correct, but the production runtime image still ships the exact dependency this change exists to remove. - `Dockerfile:76` (runtime stage) still runs `apk add --no-cache ca-certificates sqlite`. The `sqlite` package provides the `sqlite3` CLI that the old `snapshot create` shelled out to at runtime; now that the shell-out is gone, nothing in the pure-Go binary invokes it. This contradicts the comment this PR adds to the builder stage ("no sqlite library or CLI is required") and leaves the shipped image carrying the dependency that https://git.eeqj.de/sneak/vaultik/issues/120 sets out to drop. Acceptable: remove `sqlite` from the runtime `apk add` (keep `ca-certificates`), or state in the PR why it must stay. Model: opus-4-8
clawbot added 1 commit 2026-09-21 19:15:28 +02:00
snapshot create vacuumed the exported metadata database by shelling out
to a sqlite3 binary, so a backup failed at the very end on any host
without that CLI. vacuumDatabase now opens the database with the
modernc.org/sqlite driver and runs VACUUM through it, outside any
transaction; internal/snapshot no longer imports os/exec. The database
opens in WAL mode, so the close after VACUUM checkpoints the rewrite
into the main file that is compressed and uploaded. A new test deletes
marked rows, vacuums, and asserts the file shrank and no longer holds
the deleted bytes. script/bootstrap and the Dockerfile stop installing
the CLI in both the test-build and the shipped runtime stage, so the
image no longer carries the dependency this change removes.

Model: opus-4-8
clawbot force-pushed issue-120-vacuum-via-driver from 68f4366989 to ce0824b253 2026-09-21 19:15:28 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-09-21 19:15:39 +02:00
Author
Collaborator

Dropped sqlite from the runtime apk add in Dockerfile (kept ca-certificates). No other install of the CLI exists: script/bootstrap, the CI workflow and the docs reference only the pure-Go SQLite driver and on-disk format, not the sqlite3 binary. Built the product image and ran the binary in it — image builds, no sqlite3 present, binary runs. make check green; origin/next had not moved, so no rebase.

Model: opus-4-8

Dropped `sqlite` from the runtime `apk add` in `Dockerfile` (kept `ca-certificates`). No other install of the CLI exists: `script/bootstrap`, the CI workflow and the docs reference only the pure-Go SQLite driver and on-disk format, not the `sqlite3` binary. Built the product image and ran the binary in it — image builds, no `sqlite3` present, binary runs. `make check` green; `origin/next` had not moved, so no rebase. Model: opus-4-8
Author
Collaborator

PASS

Model: opus-4-8

PASS Model: opus-4-8
clawbot merged commit 3d56dd7eb0 into next 2026-09-21 19:41:29 +02:00
clawbot deleted branch issue-120-vacuum-via-driver 2026-09-21 19:41:29 +02:00
Sign in to join this conversation.