diff --git a/Dockerfile b/Dockerfile index 2ba39e3..51707b4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -22,8 +22,10 @@ FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f ARG VERSION=dev -# Install build dependencies for CGO (mattn/go-sqlite3) and sqlite3 CLI (tests) -RUN apk add --no-cache make build-base sqlite +# Build tooling: make, plus a C toolchain because `go test -race` needs cgo. +# The sqlite driver is pure Go (modernc.org/sqlite), so no sqlite library or +# CLI is required. +RUN apk add --no-cache make build-base WORKDIR /src @@ -71,7 +73,7 @@ RUN CGO_ENABLED=0 go build -ldflags "-X 'sneak.berlin/go/vaultik/internal/global # alpine:3.21, 2026-02-25 FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709 -RUN apk add --no-cache ca-certificates sqlite +RUN apk add --no-cache ca-certificates # Copy binary from builder COPY --from=builder /vaultik /usr/local/bin/vaultik diff --git a/README.md b/README.md index 9e17b9c..d9c3598 100644 --- a/README.md +++ b/README.md @@ -603,7 +603,6 @@ regardless of color setting (emoji are not color). and the pre-commit hook both run it. A `golangci-lint` installed on `PATH` is not a substitute and is never used on a host, whatever its version. -* `sqlite3` CLI, which the test suite shells out to * S3-compatible object storage (or local filesystem, or rclone remote) ## development workflow @@ -634,8 +633,8 @@ standard: normalized scripts in `script/` are the entrypoints for the development workflow, and the Makefile targets are thin shims that call them. We provide: -* `script/bootstrap` — install all development dependencies (go, sqlite3, - Go module download). It deliberately does not install `golangci-lint`; +* `script/bootstrap` — install all development dependencies (go, Go + module download). It deliberately does not install `golangci-lint`; see `script/lint` below. * `script/setup` — make a fresh clone ready for development: runs `script/bootstrap`, then `script/install-precommit` diff --git a/TODO.md b/TODO.md index 3f2ef0d..7818366 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,15 @@ release" is exactly the contradiction # Completed Steps +- 2026-09-21: Made `snapshot create` VACUUM the per-snapshot metadata + database through the `modernc.org/sqlite` driver instead of shelling + out to the external `sqlite` command-line binary (issue #120). A + backup no longer needs that binary on `PATH`, so `make check` passes + on a stock `go install` host; `script/bootstrap` and the `Dockerfile` + (both the test-build and the shipped runtime stage) no longer install + it, and a new test asserts the uploaded database keeps no pages from + deleted rows. Dropped the now-false note on the 2026-08-07 entry below + that said bootstrap installs it. - 2026-09-21: Made `.gitea/workflows/check.yml` run on pushes to `main` and `next` and on pull requests against either, so unit PRs (whose base is `next`) and `next` itself get a CI run instead of relying on a @@ -529,7 +538,7 @@ release" is exactly the contradiction was green was wrong. - 2026-08-07: Added the standard `.golangci.yml` and `.editorconfig` (issue #59); lint findings under the new config are tracked in issue - #61. `script/bootstrap` now installs sqlite3 (needed by tests). + #61. - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, Makefile shims, README Entrypoints section - 2026-07-02: Consolidated CLI verbs, retired overlapping commands; bound diff --git a/docs/DATAMODEL.md b/docs/DATAMODEL.md index 01a6dec..b581651 100644 --- a/docs/DATAMODEL.md +++ b/docs/DATAMODEL.md @@ -192,7 +192,7 @@ Tracks blob upload metrics. After a snapshot is completed: 1. Copy database to temporary file 2. Clean temporary database to contain only current snapshot data -3. Export to SQL dump using sqlite3 +3. VACUUM the trimmed database so deleted rows leave no pages behind 4. Compress with zstd and encrypt with age 5. Upload to S3 as `metadata/{snapshot-id}/db.zst.age` 6. Generate blob manifest and upload as `metadata/{snapshot-id}/manifest.json.zst` diff --git a/internal/snapshot/snapshot.go b/internal/snapshot/snapshot.go index f445e4f..f7e127f 100644 --- a/internal/snapshot/snapshot.go +++ b/internal/snapshot/snapshot.go @@ -44,7 +44,6 @@ import ( "errors" "fmt" "io" - "os/exec" "path/filepath" "strings" "time" @@ -669,14 +668,31 @@ func (sm *SnapshotManager) collectCleanupStats( // vacuumDatabase runs VACUUM on the database to remove deleted data and compact // This is critical for security - ensures no stale/deleted data pages are uploaded +// +// VACUUM runs through the modernc.org/sqlite driver, on a freshly opened +// connection with no transaction in flight (VACUUM cannot run inside one). +// The database opens in WAL mode, so VACUUM's rewrite lands in the WAL; the +// checkpoint on Close flushes it into the main file, which is the file we +// then compress and upload. func (sm *SnapshotManager) vacuumDatabase(ctx context.Context, dbPath string) error { log.Debug("Running VACUUM on database", "path", dbPath) - //nolint:gosec // G204: fixed argv; dbPath is our own temp file path - cmd := exec.CommandContext(ctx, "sqlite3", dbPath, "VACUUM;") - output, err := cmd.CombinedOutput() + db, err := database.New(ctx, dbPath) if err != nil { - return fmt.Errorf("running VACUUM: %w (output: %s)", err, string(output)) + return fmt.Errorf("opening database for VACUUM: %w", err) + } + + defer func() { + cerr := db.Close() + if cerr != nil { + log.Debug("Failed to close database after VACUUM", + "path", dbPath, "error", cerr) + } + }() + + _, err = db.ExecWithLog(ctx, "VACUUM") + if err != nil { + return fmt.Errorf("running VACUUM: %w", err) } return nil diff --git a/internal/snapshot/snapshot_test.go b/internal/snapshot/snapshot_test.go index 878366e..42061a2 100644 --- a/internal/snapshot/snapshot_test.go +++ b/internal/snapshot/snapshot_test.go @@ -2,6 +2,7 @@ package snapshot import ( + "bytes" "context" "database/sql" "io" @@ -96,6 +97,97 @@ func verifyCleanedDB( } } +// TestVacuumDatabaseRemovesDeletedData proves the export path uploads a +// compacted database: after rows carrying a recognizable marker are deleted +// and vacuumDatabase runs, no page holding that marker survives in the file +// on disk (the file compressFile later reads for upload). +func TestVacuumDatabaseRemovesDeletedData(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + ctx := context.Background() + fs := afero.NewOsFs() + + tempDir := t.TempDir() + dbPath := filepath.Join(tempDir, "snapshot.db") + + db, err := database.New(ctx, dbPath) + if err != nil { + t.Fatalf("failed to create database: %v", err) + } + + // A marker distinctive enough that its presence in the raw file can only + // come from the rows inserted below. + marker := []byte("VACUUM_PROBE_DEADBEEF_DELETED_ROW") + payload := bytes.Repeat(marker, 128) // ~4 KiB per row + + _, err = db.Conn().ExecContext(ctx, + "CREATE TABLE vacuum_probe (id INTEGER PRIMARY KEY, payload BLOB)") + if err != nil { + t.Fatalf("failed to create probe table: %v", err) + } + + for range 512 { + _, err = db.Conn().ExecContext(ctx, + "INSERT INTO vacuum_probe (payload) VALUES (?)", payload) + if err != nil { + t.Fatalf("failed to insert probe row: %v", err) + } + } + + _, err = db.Conn().ExecContext(ctx, "DELETE FROM vacuum_probe") + if err != nil { + t.Fatalf("failed to delete probe rows: %v", err) + } + + // Close so the deletes reach the main file, mirroring the state + // prepareExportDB hands to vacuumDatabase. + err = db.Close() + if err != nil { + t.Fatalf("failed to close database: %v", err) + } + + beforeInfo, err := fs.Stat(dbPath) + if err != nil { + t.Fatalf("failed to stat database before vacuum: %v", err) + } + + beforeBytes, err := afero.ReadFile(fs, dbPath) + if err != nil { + t.Fatalf("failed to read database before vacuum: %v", err) + } + + if !bytes.Contains(beforeBytes, marker) { + t.Fatalf("expected deleted-row data to linger before vacuum") + } + + sm := &SnapshotManager{fs: fs} + + err = sm.vacuumDatabase(ctx, dbPath) + if err != nil { + t.Fatalf("vacuumDatabase failed: %v", err) + } + + afterBytes, err := afero.ReadFile(fs, dbPath) + if err != nil { + t.Fatalf("failed to read database after vacuum: %v", err) + } + + if bytes.Contains(afterBytes, marker) { + t.Fatalf("deleted-row data survived vacuum in the uploaded file") + } + + afterInfo, err := fs.Stat(dbPath) + if err != nil { + t.Fatalf("failed to stat database after vacuum: %v", err) + } + + if afterInfo.Size() >= beforeInfo.Size() { + t.Fatalf("expected vacuum to shrink the file: before=%d after=%d", + beforeInfo.Size(), afterInfo.Size()) + } +} + func TestCleanSnapshotDBEmptySnapshot(t *testing.T) { // Initialize logger log.Initialize(log.Config{}) diff --git a/script/bootstrap b/script/bootstrap index 01febae..21862fe 100755 --- a/script/bootstrap +++ b/script/bootstrap @@ -114,9 +114,6 @@ main() { # from CI. Nothing on the host is ever used as a linter, at any # version, so installing one here would buy nothing. - # sqlite3 CLI: the test suite shells out to it (VACUUM). - if missing sqlite3; then pkg_install sqlite sqlite3 sqlite sqlite; fi - # goreleaser, at the version pinned by script/install-goreleaser and # verified against a hardcoded sha256. Package managers are not used # for it: they ship whatever version they happen to carry, and the