amadan

oficina / home Public

Open the database, and prove secure_delete reaches every connection

§13 step 2, Task 2, second half.

- internal/home/db.go: RegisterSecureDelete, guarded by a sync.Once
  because sqlite's hook list is process-global.
- cmd/home/main.go and newTestApp: register the hook, open the handle,
  apply BootSchema, blank DBPath before Serve so Serve does not open a
  second handle against the same file.
- App now holds *db.DB.
- newTestApp becomes a struct rather than a widening tuple. Later tasks
  add the LinkStore and the signin.Flow, and each would otherwise be a
  signature change rippling through every test already written.

Why a driver hook and not an Exec, since this was the plan's riskiest
assumption and it holds:

  secure_delete is per connection. An Exec after opening sets it on
  whichever pooled connection served that call, on no other, and on none
  opened later — and rastrillo/db.Open builds two pools. Reading the
  pragma back on that same connection passes while proving nothing.

  rastrillo/db.Open builds its own DSN (busy_timeout, journal_mode,
  foreign_keys) and passes nothing else through, so there is no way to
  set this as a DSN pragma without changing rastrillo. Verified at
  source, not assumed: sqlite.RegisterConnectionHook is exported at
  package level (sqlite.go:785), not only as a method on an unexported
  driver.

  It is registered before db.Open, because Open calls Ping, which opens
  the writer pool's single long-lived connection there and then. A hook
  registered afterwards misses that connection forever, and it is the
  only one that performs DELETEs.

The test asserts the writer AND a reader, and it was verified to fail:
flipping the pragma to OFF reports 0 on both pools.

Three database-level tests beyond the plan, each covering something the
static gate cannot see: that both table sets exist after the merge, and
that the database as actually built has no address column anywhere —
gate 2's schema half reads this repo's .sql files, so a column arriving
from a dependency's migrations would be invisible to it. The runtime
check is a deliberately separate implementation from the static one:
two independent checks that agree are worth more than one shared helper
that can be wrong in a single place.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E4uVkB6xnZJYuxTooPiUn4
Paul Campbell pushed by paul@keymail.dev 9209153885a461890dde87c454a328657535c7df parent bbe7604
8 files changed, +263 −18
  • cmd/home/main.go +28 −1
  • go.mod +2 −2
  • internal/home/app.go +6 −5
  • internal/home/app_test.go +2 −2
  • internal/home/db.go +48 −0
  • internal/home/db_test.go +137 −0
  • internal/home/logs_test.go +1 −1
  • internal/home/testapp_test.go +39 −7

Intent

full ledger: step2/seal-and-secrets

diff --git a/cmd/home/main.go b/cmd/home/main.go
index 93f9641..205fbb6 100644
--- a/cmd/home/main.go
+++ b/cmd/home/main.go
@@ -30,11 +30,14 @@
package main
import (
+ "context"
"log/slog"
"net/http"
"os"
"github.com/carlosframework/rastrillo"
+ rdb "github.com/carlosframework/rastrillo/db"
+ "github.com/carlosframework/rastrillo/migrate"
"amadan.net/oficina/home/internal/home"
)
@@ -62,7 +65,27 @@ func main() {
os.Exit(1)
}
- app := home.New(secrets)
+ // Before db.Open, never after: Open calls Ping, which opens the
+ // writer pool's single long-lived connection there and then, and a
+ // hook registered afterwards misses it forever. That connection is
+ // the only one that performs DELETEs.
+ home.RegisterSecureDelete()
+
+ d, err := rdb.Open(opts.DBPath, logger)
+ if err != nil {
+ logger.Error("open db", "err", err)
+ os.Exit(1)
+ }
+ defer d.Close()
+
+ // Ledgered, once at boot, forward-only. A boot that cannot reconcile
+ // its schema refuses rather than guessing, same as a failed open.
+ if _, err := migrate.Apply(context.Background(), d, home.BootSchema); err != nil {
+ logger.Error("migrate", "err", err)
+ os.Exit(1)
+ }
+
+ app := home.New(secrets, d)
// rastrillo.Options.Mux wants a *http.ServeMux; the app's own
// router is a chi.Router (an http.Handler), so it mounts under one.
@@ -70,6 +93,10 @@ func main() {
mux.Handle("/", app.Router())
opts.Mux = mux
+ // Blanked because this file opened the handle itself: Serve would
+ // otherwise open a second one against the same file.
+ opts.DBPath = ""
+
if err := rastrillo.Serve(opts); err != nil {
logger.Error("serve", "err", err)
os.Exit(1)
diff --git a/go.mod b/go.mod
index abc1c89..55af060 100644
--- a/go.mod
+++ b/go.mod
@@ -6,6 +6,8 @@ require (
github.com/carlosframework/rastrillo v0.22.0
github.com/go-chi/chi/v5 v5.3.2
golang.org/x/crypto v0.55.0
+ gorm.io/plugin/dbresolver v1.6.2
+ modernc.org/sqlite v1.55.0
)
require (
@@ -19,9 +21,7 @@ require (
golang.org/x/sys v0.47.0 // indirect
golang.org/x/text v0.41.0 // indirect
gorm.io/gorm v1.31.2 // indirect
- gorm.io/plugin/dbresolver v1.6.2 // indirect
modernc.org/libc v1.74.1 // indirect
modernc.org/mathutil v1.7.1 // indirect
modernc.org/memory v1.11.0 // indirect
- modernc.org/sqlite v1.55.0 // indirect
)
diff --git a/internal/home/app.go b/internal/home/app.go
index 94b8f23..918cfef 100644
--- a/internal/home/app.go
+++ b/internal/home/app.go
@@ -14,17 +14,18 @@ package home
import (
"net/http"
+ "github.com/carlosframework/rastrillo/db"
"github.com/go-chi/chi/v5"
)
// App holds the wiring. One per process.
//
-// It holds no database handle yet: storage arrives with the schema in
-// the next step of the plan. What it does hold is the validated
-// Secrets, because every value this app will persist is sealed on the
-// way in and there is no code path that should reach for a raw key.
+// It holds the validated Secrets because every value this app persists
+// is sealed on the way in, and the database handle because that is what
+// it is sealed into. Nothing here reaches for a raw key.
type App struct {
secrets Secrets
+ db *db.DB
}
// New builds an App on validated secrets.
@@ -34,7 +35,7 @@ type App struct {
// addresses in the clear, which is the failure this whole step exists
// to make impossible. Callers get their Secrets from LoadSecrets, which
// refuses rather than degrades.
-func New(secrets Secrets) *App { return &App{secrets: secrets} }
+func New(secrets Secrets, d *db.DB) *App { return &App{secrets: secrets, db: d} }
// Router returns the mounted handler.
func (a *App) Router() http.Handler {
diff --git a/internal/home/app_test.go b/internal/home/app_test.go
index e2b3b25..76b5755 100644
--- a/internal/home/app_test.go
+++ b/internal/home/app_test.go
@@ -10,7 +10,7 @@ import (
// HTML — the placeholder content itself is not asserted here so this
// test does not need updating every time the wording changes.
func TestIndex(t *testing.T) {
- app, _ := newTestApp(t)
+ app := newTestApp(t)
req := httptest.NewRequest(http.MethodGet, "/", nil)
rec := httptest.NewRecorder()
app.Router().ServeHTTP(rec, req)
@@ -28,7 +28,7 @@ func TestIndex(t *testing.T) {
// ahead of this one and wins (see the comment on Router in app.go); this
// test only proves the app's router itself is wired and reachable.
func TestHealthz(t *testing.T) {
- app, _ := newTestApp(t)
+ app := newTestApp(t)
req := httptest.NewRequest(http.MethodGet, "/healthz", nil)
rec := httptest.NewRecorder()
app.Router().ServeHTTP(rec, req)
diff --git a/internal/home/db.go b/internal/home/db.go
new file mode 100644
index 0000000..8d3ae18
--- /dev/null
+++ b/internal/home/db.go
@@ -0,0 +1,48 @@
+package home
+
+import (
+ "context"
+ "sync"
+
+ "modernc.org/sqlite"
+)
+
+// secureDeleteOnce guards the connection hook registration. The hook
+// list is process-global — sqlite.RegisterConnectionHook appends to the
+// package's single driver — so registering per call would run the pragma
+// once per registration on every new connection, forever.
+var secureDeleteOnce sync.Once
+
+// RegisterSecureDelete turns on PRAGMA secure_delete for every
+// connection this process opens.
+//
+// It matters because deleting a row does not remove its bytes from a
+// file that is continuously replicated (§5.1): without it, a redeemed
+// magic link's sealed row is gone from the table and still present in
+// the file, and in every WAL frame Litestream has already shipped.
+//
+// Three things make this the shape it is, and each was a way to get it
+// wrong:
+//
+// The pragma is PER CONNECTION. An Exec after opening sets it on
+// whichever pooled connection served that call and on no other, and on
+// none opened later. A test that reads the pragma back on that same
+// connection passes while proving nothing.
+//
+// rastrillo/db.Open builds its own DSN — busy_timeout, journal_mode and
+// foreign_keys — and passes nothing else through, so there is no way to
+// add secure_delete as a DSN pragma without changing rastrillo. Hence
+// the driver hook.
+//
+// It must be registered BEFORE db.Open. Open calls Ping, which opens the
+// writer pool's single long-lived connection there and then; a hook
+// registered afterwards misses that connection forever, and it is the
+// only one that performs DELETEs.
+func RegisterSecureDelete() {
+ secureDeleteOnce.Do(func() {
+ sqlite.RegisterConnectionHook(func(conn sqlite.ExecQuerierContext, _ string) error {
+ _, err := conn.ExecContext(context.Background(), "PRAGMA secure_delete=ON", nil)
+ return err
+ })
+ })
+}
diff --git a/internal/home/db_test.go b/internal/home/db_test.go
new file mode 100644
index 0000000..aed3685
--- /dev/null
+++ b/internal/home/db_test.go
@@ -0,0 +1,137 @@
+package home
+
+import (
+ "testing"
+
+ "gorm.io/plugin/dbresolver"
+)
+
+// TestSecureDeleteIsOnEveryConnection is the point of the connection
+// hook, and the reason it is a hook at all.
+//
+// It asserts on the writer AND on a reader. The pragma is per
+// connection, and rastrillo/db.Open builds two pools — a writer capped
+// at one, and readers. A test that checked only whichever connection it
+// happened to get would pass while the other pool had no pragma set,
+// which is the worst kind of green. The writer is the connection that
+// matters most, because it is the only one that performs DELETEs, and it
+// is also the one opened during Open's Ping, before any Exec a caller
+// could run.
+func TestSecureDeleteIsOnEveryConnection(t *testing.T) {
+ app := newTestApp(t)
+
+ var writer int
+ if err := app.db.Writer().QueryRow("PRAGMA secure_delete").Scan(&writer); err != nil {
+ t.Fatalf("read secure_delete on the writer: %v", err)
+ }
+ if writer != 1 {
+ t.Errorf("secure_delete on the writer = %d, want 1", writer)
+ }
+
+ var reader int
+ if err := app.db.G.Clauses(dbresolver.Read).Raw("PRAGMA secure_delete").Scan(&reader).Error; err != nil {
+ t.Fatalf("read secure_delete on a reader: %v", err)
+ }
+ if reader != 1 {
+ t.Errorf("secure_delete on a reader = %d, want 1", reader)
+ }
+}
+
+// The schema applied at boot must be the one the app declares, and it
+// must contain the sessions tables as well as this app's own. A merge
+// that silently dropped one would show up here and nowhere else until
+// the first sign-in.
+func TestBootSchemaCreatesBothTableSets(t *testing.T) {
+ app := newTestApp(t)
+
+ for _, table := range []string{"people", "sessions"} {
+ var name string
+ err := app.db.Writer().
+ QueryRow("SELECT name FROM sqlite_master WHERE type='table' AND name=?", table).
+ Scan(&name)
+ if err != nil {
+ t.Errorf("table %q was not created: %v", table, err)
+ }
+ }
+}
+
+// There is no address column, and this asserts it against the database
+// that was actually built rather than against the migration text. Gate
+// 2's schema half reads the .sql files; this reads sqlite_master, so a
+// column arriving from a dependency's migrations — rastrillo/auth's, if
+// anybody ever merged its schema — is caught too.
+func TestNoAddressColumnInTheBuiltDatabase(t *testing.T) {
+ app := newTestApp(t)
+
+ rows, err := app.db.Writer().Query("SELECT name FROM sqlite_master WHERE type='table'")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer rows.Close()
+
+ var tables []string
+ for rows.Next() {
+ var name string
+ if err := rows.Scan(&name); err != nil {
+ t.Fatal(err)
+ }
+ tables = append(tables, name)
+ }
+ if err := rows.Err(); err != nil {
+ t.Fatal(err)
+ }
+
+ for _, table := range tables {
+ cols, err := app.db.Writer().Query("SELECT name FROM pragma_table_info(?)", table)
+ if err != nil {
+ t.Fatalf("pragma_table_info(%q): %v", table, err)
+ }
+ for cols.Next() {
+ var col string
+ if err := cols.Scan(&col); err != nil {
+ cols.Close()
+ t.Fatal(err)
+ }
+ if addressishColumn(col) {
+ t.Errorf("table %q has column %q, which names an address", table, col)
+ }
+ }
+ cols.Close()
+ }
+}
+
+// addressishColumn is the runtime twin of gate 2's static matcher. It is
+// deliberately a separate, simpler implementation: two independent
+// checks that agree are worth more than one shared helper that could be
+// wrong in one place.
+func addressishColumn(col string) bool {
+ for _, part := range splitIdent(col) {
+ switch part {
+ case "address", "email", "mail", "recipient":
+ return true
+ }
+ }
+ return false
+}
+
+func splitIdent(s string) []string {
+ var out []string
+ cur := ""
+ for _, r := range s {
+ if r == '_' || r == '-' {
+ if cur != "" {
+ out = append(out, cur)
+ }
+ cur = ""
+ continue
+ }
+ if r >= 'A' && r <= 'Z' {
+ r += 'a' - 'A'
+ }
+ cur += string(r)
+ }
+ if cur != "" {
+ out = append(out, cur)
+ }
+ return out
+}
diff --git a/internal/home/logs_test.go b/internal/home/logs_test.go
index 2ddc2c3..5a1d493 100644
--- a/internal/home/logs_test.go
+++ b/internal/home/logs_test.go
@@ -59,7 +59,7 @@ func TestNoContentInLogs(t *testing.T) {
log.SetFlags(prevFlags)
})
- app, _ := newTestApp(t)
+ app := newTestApp(t)
router := app.Router()
routes, ok := router.(chi.Routes)
diff --git a/internal/home/testapp_test.go b/internal/home/testapp_test.go
index 1c2acb6..1e2f85e 100644
--- a/internal/home/testapp_test.go
+++ b/internal/home/testapp_test.go
@@ -2,9 +2,13 @@ package home
import (
"context"
+ "path/filepath"
"sync"
"testing"
+ "github.com/carlosframework/rastrillo/db"
+ "github.com/carlosframework/rastrillo/migrate"
+
"amadan.net/oficina/home/internal/home/seal"
)
@@ -68,14 +72,42 @@ func testSecrets(t *testing.T) (Secrets, *captureMailer) {
}, mailer
}
-// newTestApp builds an App on test secrets.
+// testApp is an App plus the handles a test needs to look behind it: the
+// mail it would have sent, and the database file itself, which gate 1
+// scans for plaintext.
+//
+// A struct rather than a widening tuple: later steps add the LinkStore
+// and the signin.Flow, and every one of those would otherwise be a
+// signature change rippling through every test already written.
+type testApp struct {
+ *App
+ mailer *captureMailer
+ db *db.DB
+ dbPath string
+}
+
+// newTestApp builds an App on test secrets against a fresh database.
//
-// It starts small and grows. Later steps of the plan add the database
-// and its migrations, the LinkStore, and the signin.Flow; each extends
-// this helper rather than replacing it, so every test written before
-// then keeps compiling.
-func newTestApp(t *testing.T) (*App, *captureMailer) {
+// The connection hook is registered before db.Open, exactly as main does
+// and for the same reason: Open pings, which opens the writer pool's one
+// long-lived connection, and a hook registered after that misses the
+// only connection that ever deletes anything.
+func newTestApp(t *testing.T) *testApp {
t.Helper()
secrets, mailer := testSecrets(t)
- return New(secrets), mailer
+
+ RegisterSecureDelete()
+
+ dbPath := filepath.Join(t.TempDir(), "home.db")
+ d, err := db.Open(dbPath, quietLogger())
+ if err != nil {
+ t.Fatalf("db.Open: %v", err)
+ }
+ t.Cleanup(func() { d.Close() })
+
+ if _, err := migrate.Apply(context.Background(), d, BootSchema); err != nil {
+ t.Fatalf("migrate.Apply: %v", err)
+ }
+
+ return &testApp{App: New(secrets, d), mailer: mailer, db: d, dbPath: dbPath}
}