Create every SQLite file 0600 (closes #255)

webhooker.db holds target configuration in plaintext — bearer tokens,
API keys, Slack webhook URLs — and the session encryption key, and it
was created 0644. The 0750 data directory was therefore the only
barrier, and a Docker bind mount supplies that directory at 0755,
which removes it: every local user on the host could read every stored
credential.

The mode is settled in OpenSQLite, the single open path all three
tiers share, so the main database, the per-webhook event databases and
the archive databases are covered in one place.

0644 comes from SQLite itself: robust_open substitutes
SQLITE_DEFAULT_FILE_PERMISSIONS whenever it is handed mode 0, and
findCreateFileMode yields 0 for a main database opened by URI with no
`modeof` parameter. A chmod after opening would leave a window in
which the credentials are on disk world-readable, so OpenSQLite
creates the file itself at 0600 before the driver sees the path.

That also settles the WAL sidecars, which carry the same rows and
would otherwise leave the fix worthless. SQLite derives both from the
main database file — `-wal` through findCreateFileMode, which stats
the path with the suffix stripped, and `-shm` in unixOpenSharedMemory
from an fstat of the open database descriptor — so a main file at 0600
produces sidecars at 0600. Verified by stat rather than by reading the
driver: a pre-change build leaves webhooker.db, -wal and -shm all 644;
this one leaves all three 600, in a 0755 bind mount, for all three
tiers, with delivery and restart working.

Existing files are chmodded on open, so a directory an earlier build
left 0644 — including a developer's scratch directory — is fixed
without any migration machinery.

This is not encryption at rest. An unattended process needs a key it
can read without a human, so the key lands beside the data and an
attacker who can read the database can read it too.

The data directory stays 0750: the group bit may matter to a
deployment, and with the files at 0600 the directory is no longer the
barrier.

README: the Docker section now states the DATA_DIR ownership
requirement where the bind mount is documented. A `-v` source path
Docker creates is root:root, and the container runs as UID 1000, so it
fails to start on the lock file; the chown that fixes it appeared only
under Restore.

README.md

@@ -686,6 +686,44 @@ databases written by `database` targets (`archive-{uuid}.db`). Mount
 this as a persistent volume to preserve data across container
 restarts.
 
+**The bind-mounted directory must be owned by UID 1000, or the
+container does not start.** Docker creates a `-v` source path that
+does not exist yet as `root:root`, and the process runs as UID 1000,
+so it cannot take its `DATA_DIR` lock:
+
+```
+webhooker: locking data directory /var/lib/webhooker: open
+/var/lib/webhooker/webhooker.lock: permission denied
+```
+
+It exits non-zero at that point, before opening any database. Create
+the directory ahead of the first `docker run`:
+
+```bash
+mkdir -p /path/to/data
+chown 1000:1000 /path/to/data
+chmod 750 /path/to/data
+```
+
+The same `chown` is what a restore needs — see step 4 of
+[Restore](#restore). A **named volume** does not have this problem:
+Docker copies the image's ownership onto a volume it initializes, and
+the image creates `/var/lib/webhooker` owned by `webhooker`.
+
+**The file modes are not yours to set, and do not depend on the
+directory.** `webhooker.db` holds target configuration in plaintext —
+bearer tokens, API keys, Slack webhook URLs — along with the session
+encryption key, so webhooker creates every SQLite file it owns `0600`:
+each database and both of its `-wal` and `-shm` sidecars, across all
+three tiers. Files an earlier build left `0644` are tightened when
+they are opened. A `DATA_DIR` webhooker creates itself is `0750`, but
+a bind mount supplies its own directory and Docker's default for one
+it creates is `0755`; the `0600` files hold there regardless. The
+`chmod 750` above is defence in depth — it stops other local users
+listing the directory and learning your webhook UUIDs from the
+`events-{uuid}.db` filenames — not the barrier protecting the
+credentials.
+
 ## Deployment behind a reverse proxy
 
 webhooker terminates no TLS of its own. It serves plaintext HTTP and
@@ -1642,6 +1680,11 @@ webhooker uses **separate SQLite database files**: a main application
 database for configuration data and per-webhook databases for event
 storage. All database files live in the `DATA_DIR` directory.
 
+Every one of them is created `0600`, and so is each `-wal` and `-shm`
+sidecar. See
+[Running with Docker](#running-with-docker) for what that does and
+does not protect.
+
 **Main Application Database** (`{DATA_DIR}/webhooker.db`) — stores
 configuration and application state:
 

internal/database/sqlite_mode_test.go

@@ -0,0 +1,240 @@
+package database_test
+
+import (
+	"context"
+	"io/fs"
+	"net/http"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/fx/fxtest"
+	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/globals"
+	"sneak.berlin/go/webhooker/internal/logger"
+)
+
+// ownerOnly is the mode every SQLite file the service owns must have.
+// Spelled out rather than referencing database.SQLiteFilePerm so the
+// test fails if the constant itself is loosened.
+const ownerOnly fs.FileMode = 0o600
+
+// requireOwnerOnly asserts that path exists and is readable and
+// writable by its owner and by nobody else.
+func requireOwnerOnly(t *testing.T, path string) {
+	t.Helper()
+
+	info, err := os.Stat(path)
+	require.NoError(t, err, "%s must exist", path)
+	assert.Equal(
+		t,
+		ownerOnly,
+		info.Mode().Perm(),
+		"%s holds credentials and must not be readable by "+
+			"anyone but its owner",
+		path,
+	)
+}
+
+// requireDatabaseSetOwnerOnly asserts the mode of a database file and
+// of both WAL sidecars. The sidecars carry the same rows as the
+// database, so tightening only the main file fixes nothing.
+func requireDatabaseSetOwnerOnly(t *testing.T, dbPath string) {
+	t.Helper()
+
+	requireOwnerOnly(t, dbPath)
+	requireOwnerOnly(t, dbPath+"-wal")
+	requireOwnerOnly(t, dbPath+"-shm")
+}
+
+// TestMainDatabaseFilesAreOwnerOnly covers the tier the defect was
+// reported against: webhooker.db holds targets.config in plaintext —
+// bearer tokens, API keys, Slack webhook URLs — and the session
+// encryption key.
+func TestMainDatabaseFilesAreOwnerOnly(t *testing.T) {
+	t.Parallel()
+
+	lc := fxtest.NewLifecycle(t)
+
+	l, err := logger.New(lc, logger.LoggerParams{
+		Globals: &globals.Globals{
+			Appname: testAppname,
+			Version: testVersion,
+		},
+	})
+	require.NoError(t, err)
+
+	// A directory the application creates itself, not one t.TempDir
+	// made at 0700, so the mode below is the application's.
+	dataDir := filepath.Join(t.TempDir(), "data")
+
+	db, err := database.New(lc, database.DatabaseParams{
+		Config: &config.Config{DataDir: dataDir},
+		Logger: l,
+	})
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	require.NoError(t, lc.Start(ctx))
+
+	defer func() { require.NoError(t, lc.Stop(ctx)) }()
+
+	// Write through the real model so the WAL is populated and both
+	// sidecars are on disk while the handle is open.
+	require.NoError(t, db.DB().Create(&database.Webhook{
+		Name: testWebhookName,
+	}).Error)
+
+	requireDatabaseSetOwnerOnly(
+		t, filepath.Join(dataDir, database.MainDBFileName),
+	)
+
+	// The data directory grants nothing to `other`. Asserted as a
+	// property rather than as an exact 0750, because MkdirAll applies
+	// the ambient umask: the exact mode is the developer's umask as
+	// much as the application's request, and pinning it would make
+	// `make check` pass or fail on where it is run. The group bits are
+	// deliberately left unasserted — deployments may rely on them.
+	info, err := os.Stat(dataDir)
+	require.NoError(t, err)
+	assert.Zero(
+		t,
+		info.Mode().Perm()&0o007,
+		"the data directory must not be world-accessible",
+	)
+}
+
+// TestPerWebhookEventDatabaseFilesAreOwnerOnly covers the events-*.db
+// tier. These carry no credential canaries since
+// https://git.eeqj.de/sneak/webhooker/issues/206, but they hold every
+// received request body and header.
+func TestPerWebhookEventDatabaseFilesAreOwnerOnly(t *testing.T) {
+	t.Parallel()
+
+	mgr, lc := setupTestWebhookDBManager(t)
+	ctx := context.Background()
+	require.NoError(t, lc.Start(ctx))
+
+	defer func() { require.NoError(t, lc.Stop(ctx)) }()
+
+	webhookID := uuid.New().String()
+
+	db, err := mgr.GetDB(webhookID)
+	require.NoError(t, err)
+
+	require.NoError(t, db.Create(&database.Event{
+		WebhookID:    webhookID,
+		EntrypointID: uuid.New().String(),
+		Method:       http.MethodPost,
+		Body:         "{}",
+	}).Error)
+
+	requireDatabaseSetOwnerOnly(t, mgr.DBPath(webhookID))
+}
+
+// TestArchiveDatabaseFilesAreOwnerOnly covers the archive-*.db tier.
+// internal/delivery builds that path and opens it through OpenSQLite,
+// the same single open path exercised here, so the mode is settled for
+// all three tiers in one place.
+func TestArchiveDatabaseFilesAreOwnerOnly(t *testing.T) {
+	t.Parallel()
+
+	ctx := context.Background()
+	path := filepath.Join(
+		t.TempDir(), "archive-"+uuid.New().String()+".db",
+	)
+
+	sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
+	require.NoError(t, err)
+
+	defer func() { require.NoError(t, sqlDB.Close()) }()
+
+	_, err = sqlDB.ExecContext(ctx, "create table t (id integer)")
+	require.NoError(t, err)
+
+	requireDatabaseSetOwnerOnly(t, path)
+}
+
+// TestOpenSQLiteTightensFilesLeftWorldReadable is the upgrade case: a
+// data directory an earlier build left at 0644, including a
+// developer's own scratch directory, is fixed when it is opened rather
+// than staying exposed until it is recreated.
+func TestOpenSQLiteTightensFilesLeftWorldReadable(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	path := filepath.Join(dir, database.MainDBFileName)
+
+	// A database and both sidecars as the pre-fix build left them.
+	for _, p := range []string{path, path + "-wal", path + "-shm"} {
+		require.NoError(t, os.WriteFile(p, nil, 0o644)) //nolint:gosec // the mode under test
+	}
+
+	sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
+	require.NoError(t, err)
+
+	require.NoError(t, sqlDB.Close())
+
+	requireDatabaseSetOwnerOnly(t, path)
+}
+
+// TestOpenSQLiteExistingModeDoesNotCreateTheFile guards the mechanism
+// the fix uses: OpenSQLite now creates the database file itself, and
+// must not do so for a caller that asked for an existing database. An
+// empty file materialized here would turn a missing-database error
+// into a silently empty one.
+func TestOpenSQLiteExistingModeDoesNotCreateTheFile(t *testing.T) {
+	t.Parallel()
+
+	ctx := context.Background()
+	path := filepath.Join(t.TempDir(), "absent.db")
+
+	sqlDB, err := database.OpenSQLite(path, database.SQLiteModeExisting)
+	if err == nil {
+		// sql.Open is lazy: force the connection that fails.
+		require.Error(t, sqlDB.PingContext(ctx))
+		require.NoError(t, sqlDB.Close())
+	}
+
+	_, statErr := os.Stat(path)
+	assert.ErrorIs(t, statErr, fs.ErrNotExist)
+}
+
+// TestReopenAfterRestartKeepsFilesOwnerOnly is the restart case: a
+// process that closed its files must be able to open them again at
+// 0600, including through a gorm handle, and the sidecars must come
+// back at 0600 too rather than at SQLite's own default.
+func TestReopenAfterRestartKeepsFilesOwnerOnly(t *testing.T) {
+	t.Parallel()
+
+	ctx := context.Background()
+	dir := t.TempDir()
+	path := filepath.Join(dir, database.MainDBFileName)
+
+	first, err := database.OpenSQLite(path, database.SQLiteModeCreate)
+	require.NoError(t, err)
+
+	_, err = first.ExecContext(ctx, "create table t (id integer)")
+	require.NoError(t, err)
+	require.NoError(t, first.Close())
+
+	second, err := database.OpenSQLite(path, database.SQLiteModeCreate)
+	require.NoError(t, err)
+
+	defer func() { require.NoError(t, second.Close()) }()
+
+	_, err = second.ExecContext(ctx, "insert into t (id) values (1)")
+	require.NoError(t, err)
+
+	requireDatabaseSetOwnerOnly(t, path)
+
+	var got int
+
+	require.NoError(t,
+		second.QueryRowContext(ctx, "select id from t").Scan(&got))
+	assert.Equal(t, 1, got)
+}

internal/database/sqlite_open.go

@@ -2,8 +2,11 @@ package database
 
 import (
 	"database/sql"
+	"errors"
 	"fmt"
+	"io/fs"
 	"net/url"
+	"os"
 	"time"
 
 	_ "modernc.org/sqlite" // Pure Go SQLite driver
@@ -72,6 +75,90 @@ const (
 	sqliteConnMaxIdleTime = time.Minute
 )
 
+// SQLiteFilePerm is the mode every SQLite file this service owns is
+// created with and held at: owner read/write, nothing for group or
+// other.
+//
+// These files hold credentials in plaintext. The main database stores
+// `targets.config` — bearer tokens, API keys, Slack webhook URLs — and
+// the session encryption key. SQLite left to itself creates them 0644
+// (see reserveSQLiteFile), which made the 0750 data directory the only
+// barrier; a bind-mounted directory supplied at 0755 removes it and
+// every local user on the host can read every stored credential.
+//
+// This is a file-mode fix and not encryption at rest. An unattended
+// process needs a key it can read without a human, so the key lands
+// beside the data and an attacker who can read the database can read
+// it too. See https://git.eeqj.de/sneak/webhooker/issues/212.
+const SQLiteFilePerm fs.FileMode = 0o600
+
+// reserveSQLiteFile puts path at SQLiteFilePerm before the driver ever
+// touches it, and tightens any sidecar already on disk.
+//
+// The mode has to be settled here rather than by a chmod after opening,
+// because SQLite picks it: robust_open substitutes
+// SQLITE_DEFAULT_FILE_PERMISSIONS (0644) whenever it is handed mode 0,
+// and findCreateFileMode yields 0 for a main database opened by URI
+// with no `modeof` parameter. A chmod afterwards would leave a window
+// in which the credentials are on disk world-readable.
+//
+// Creating the file ourselves also settles the sidecars, which is the
+// half that could quietly not work. SQLite does not create those at a
+// mode we choose — it derives both from the main database file:
+// `-wal` through findCreateFileMode, which stats the path with the
+// suffix stripped, and `-shm` in unixOpenSharedMemory from an fstat of
+// the already-open database descriptor. A main file at 0600 therefore
+// produces sidecars at 0600. A zero-length file is a valid empty
+// database, so reserving it changes nothing else.
+//
+// create says whether the caller is opening in a mode that may create
+// the database. When it is false a missing file is left missing, so
+// SQLite still reports the absence rather than this function
+// materializing an empty database the caller asked not to create.
+//
+// Chmod of a file that already exists is what tightens a data
+// directory an earlier build left at 0644 — including a developer's
+// own scratch directory — without any migration machinery.
+func reserveSQLiteFile(path string, create bool) error {
+	if create {
+		// gosec G304: the path is the database file the caller asked
+		// to open, and the driver is about to open the same path
+		// anyway. Creating it here is what fixes its mode.
+		f, err := os.OpenFile( //nolint:gosec // see above
+			path, os.O_RDWR|os.O_CREATE, SQLiteFilePerm,
+		)
+		if err != nil {
+			return fmt.Errorf("creating %s: %w", path, err)
+		}
+
+		err = f.Close()
+		if err != nil {
+			return fmt.Errorf("closing %s: %w", path, err)
+		}
+	}
+
+	// O_CREATE leaves an existing file's mode alone, and umask can only
+	// have narrowed a new one. Chmod settles both cases at exactly
+	// SQLiteFilePerm.
+	for _, p := range append(
+		[]string{path}, sqliteSidecarPaths(path)...,
+	) {
+		err := os.Chmod(p, SQLiteFilePerm)
+		if err != nil && !errors.Is(err, fs.ErrNotExist) {
+			return fmt.Errorf("securing %s: %w", p, err)
+		}
+	}
+
+	return nil
+}
+
+// sqliteSidecarPaths returns the files SQLite maintains beside a
+// database under WAL. They carry the same rows as the database itself,
+// so a fix that tightens only the main file has fixed nothing.
+func sqliteSidecarPaths(path string) []string {
+	return []string{path + "-wal", path + "-shm"}
+}
+
 // SQLiteDSN builds the connection string for one database file.
 //
 // mode is the SQLite URI open mode: "rwc" to create the file when it
@@ -138,9 +225,17 @@ func SQLiteDSN(path, mode string) string {
 // durability settings and pool bounds applied. mode is the SQLite URI
 // open mode ("rwc" or "rw").
 //
+// The file and its WAL sidecars are settled at SQLiteFilePerm before
+// the driver sees the path; see reserveSQLiteFile.
+//
 // The handle is returned rather than a *gorm.DB because the callers
 // wrap it in gorm themselves with their own logger.
 func OpenSQLite(path, mode string) (*sql.DB, error) {
+	err := reserveSQLiteFile(path, mode == SQLiteModeCreate)
+	if err != nil {
+		return nil, err
+	}
+
 	sqlDB, err := sql.Open("sqlite", SQLiteDSN(path, mode))
 	if err != nil {
 		return nil, fmt.Errorf(