README.md

@@ -127,6 +127,40 @@ empty string, and quietly withholding it would deny an endpoint that
 was asked for. The `hasMetricsAuth` field in the startup log and the
 existence of the route are the same value, so they cannot disagree.
 
+#### Single-instance lock
+
+Exactly one webhooker process may use a `DATA_DIR` at a time. Two
+processes sharing one open the same databases and each run delivery
+recovery over the same rows, so every pending delivery goes out twice —
+duplicate delivery to your endpoints, from nothing worse than an
+overlapping deploy or a double start.
+
+At startup, before anything opens a database, the process takes an
+exclusive advisory lock (`flock(2)`) on `{DATA_DIR}/webhooker.lock` and
+holds it for its lifetime. A second process pointed at the same
+directory prints a message naming it and exits non-zero:
+
+```
+webhooker: data directory is already in use by another instance: /var/lib/webhooker (/var/lib/webhooker/webhooker.lock). Only one webhooker may use a data directory: two both run delivery recovery over the same rows and both deliver
+```
+
+The lock is the kernel's, not the file's: it is released when the
+process exits, including `kill -9`, so a leftover `webhooker.lock`
+never blocks a restart and must not be deleted by hand. The file is
+also left in place on a clean shutdown, deliberately — unlinking it
+would let the next process lock a fresh inode while a third still held
+the old one.
+
+To run two webhookers on one host, give each its own `DATA_DIR`.
+
+`flock(2)` is host-local and per-inode: it arbitrates between processes
+and containers sharing a volume or bind mount on one machine, but not
+between hosts on a network filesystem, and a `DATA_DIR` inside a
+container's own writable layer is not shared with anything. On a
+filesystem that refuses `flock` outright, startup fails closed — the
+process reports the error and refuses to start rather than running
+unlocked.
+
 #### Trusted proxies
 
 `TRUSTED_PROXIES` is a comma-separated list of CIDR blocks (a bare
@@ -295,10 +329,12 @@ is both the simplest and the only complete rule:
 
 `{webhook_uuid}` is the webhook's UUID primary key in its canonical
 36-character hyphenated form, so a real filename looks like
-`events-3f2a1c9e-....db`. Nothing else is written to `DATA_DIR`, and no
+`events-3f2a1c9e-....db`. The only other file is `webhooker.lock`, the
-`-wal` or `-shm` files are produced (see below); a transient
+always-empty [single-instance lock](#single-instance-lock); it holds no
-`{name}.db-journal` may exist beside a database while a write is in
+state and is not part of the backup set — a copied one is stale and
-flight and is not part of the backup set.
+blocks nothing. No `-wal` or `-shm` files are produced (see below); a
+transient `{name}.db-journal` may exist beside a database while a write
+is in flight and is not part of the backup set either.
 
 Configuration is **not** in `DATA_DIR` — it comes from the environment
 and from a `.env` file read out of the process working directory. Back
@@ -1721,7 +1757,7 @@ imports. The entry point is `cmd/webhooker/main.go`.
 ```
 webhooker/
 ├── cmd/webhooker/
-│   └── main.go               # Entry point: sets globals, wires fx
+│   └── main.go               # Entry point: sets globals, locks DATA_DIR, wires fx
 ├── internal/
 │   ├── config/
 │   │   └── config.go          # Configuration loading from environment variables
@@ -1742,6 +1778,8 @@ webhooker/
 │   │   ├── retention.go       # Retention reaper (per-webhook event expiry)
 │   │   ├── testing.go         # NewTestDatabase: wrapper for tests, no fx lifecycle
 │   │   └── webhook_db_manager.go  # Per-webhook DB lifecycle manager
+│   ├── datadir/
+│   │   └── lock.go            # Exclusive advisory lock on DATA_DIR (one instance)
 │   ├── globals/
 │   │   └── globals.go         # Build-time variables (appname, version, arch)
 │   ├── gormlog/

cmd/webhooker/main.go

@@ -2,11 +2,15 @@
 package main
 
 import (
+	"fmt"
+	"io"
+	"os"
 	"time"
 
 	"go.uber.org/fx"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/datadir"
 	"sneak.berlin/go/webhooker/internal/delivery"
 	"sneak.berlin/go/webhooker/internal/globals"
 	"sneak.berlin/go/webhooker/internal/handlers"
@@ -56,7 +60,32 @@ func main() {
 	globals.Appname = appname
 	globals.Version = version
 
+	os.Exit(run(os.Stderr))
+}
+
+// run takes the exclusive DATA_DIR lock, then runs the application
+// under it, and returns the process exit status.
+//
+// The lock is taken here rather than inside the fx graph because it has
+// to be held before anything opens a database, and because a refusal
+// has to reach the operator as a plain line on standard error rather
+// than as one entry in an fx failure dump. It is released by the defer
+// on a clean shutdown, and by the kernel closing the descriptor on any
+// other exit — including the one fx performs itself when a start or
+// stop hook fails, which skips deferred calls.
+func run(stderr io.Writer) int {
+	lock, err := datadir.Acquire(config.DataDir())
+	if err != nil {
+		_, _ = fmt.Fprintf(stderr, "%s: %v\n", appname, err)
+
+		return 1
+	}
+
+	defer func() { _ = lock.Release() }()
+
 	newApp().Run()
+
+	return 0
 }
 
 // newApp builds the application graph. It is separate from main so

cmd/webhooker/main_test.go

@@ -1,10 +1,13 @@
 package main
 
 import (
+	"bytes"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/datadir"
 	"sneak.berlin/go/webhooker/internal/server"
 )
 
@@ -33,6 +36,38 @@ func TestNewApp_StopTimeout(t *testing.T) {
 	require.Less(t, got, dockerStopGrace)
 }
 
+// TestRunRefusesLockedDataDir pins what an operator's second start
+// does. The entry point must refuse before it builds the fx graph —
+// nothing may open a database in a DATA_DIR another process holds —
+// and must exit non-zero with a message naming the directory rather
+// than starting a second delivery engine over the same rows.
+//
+// flock(2) locks descriptors independently, so holding the lock here
+// is the same denial a separate process gets; internal/datadir pins
+// that property and covers the real two-process case.
+func TestRunRefusesLockedDataDir(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("DATA_DIR", dir)
+
+	lock, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+
+	defer func() { _ = lock.Release() }()
+
+	var stderr bytes.Buffer
+
+	code := run(&stderr)
+
+	require.Equal(
+		t, 1, code, "a second instance must exit non-zero",
+	)
+	assert.Contains(
+		t, stderr.String(), dir,
+		"the refusal must name the directory",
+	)
+	assert.Contains(t, stderr.String(), "another instance")
+}
+
 // tailHeadroom is the slack the fx stop budget must keep beyond the
 // server stop hook. The hooks that run after the server — the
 // delivery engine, the healthcheck, the webhook DB manager and the

go.mod

@@ -8,13 +8,14 @@ require (
 	github.com/go-chi/chi v1.5.5
 	github.com/go-chi/cors v1.2.1
 	github.com/go-chi/httprate v0.15.0
+	github.com/gofrs/flock v0.13.0
 	github.com/google/uuid v1.6.0
 	github.com/gorilla/csrf v1.7.3
 	github.com/gorilla/sessions v1.4.0
 	github.com/joho/godotenv v1.5.1
 	github.com/prometheus/client_golang v1.18.0
 	github.com/slok/go-http-metrics v0.11.0
-	github.com/stretchr/testify v1.8.4
+	github.com/stretchr/testify v1.11.1
 	go.uber.org/fx v1.20.1
 	golang.org/x/crypto v0.38.0
 	gopkg.in/yaml.v3 v3.0.1
@@ -49,7 +50,7 @@ require (
 	go.uber.org/zap v1.23.0 // indirect
 	golang.org/x/mod v0.17.0 // indirect
 	golang.org/x/sync v0.14.0 // indirect
-	golang.org/x/sys v0.33.0 // indirect
+	golang.org/x/sys v0.37.0 // indirect
 	golang.org/x/text v0.25.0 // indirect
 	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	google.golang.org/protobuf v1.31.0 // indirect

go.sum

@@ -23,6 +23,8 @@ github.com/go-chi/httprate v0.15.0 h1:j54xcWV9KGmPf/X4H32/aTH+wBlrvxL7P+SdnRqxh5
 github.com/go-chi/httprate v0.15.0/go.mod h1:rzGHhVrsBn3IMLYDOZQsSU4fJNWcjui4fWKJcCId1R4=
 github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=
 github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og=
+github.com/gofrs/flock v0.13.0 h1:95JolYOvGMqeH31+FC7D2+uULf6mG61mEZ/A8dRYMzw=
+github.com/gofrs/flock v0.13.0/go.mod h1:jxeyy9R1auM5S6JYDBhDt+E2TCo7DkratH4Pgi8P+Z0=
 github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
@@ -81,11 +83,11 @@ github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncj
 github.com/slok/go-http-metrics v0.11.0 h1:ABJUpekCZSkQT1wQrFvS4kGbhea/w6ndFJaWJeh3zL0=
 github.com/slok/go-http-metrics v0.11.0/go.mod h1:ZGKeYG1ET6TEJpQx18BqAJAvxw9jBAZXCHU7bWQqqAc=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/objx v0.5.1 h1:4VhoImhV/Bm0ToFkXFi8hXNXwpDRZ/ynw3amt82mzq0=
+github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
-github.com/stretchr/objx v0.5.1/go.mod h1:/iHQpkQwBD6DLUmQ4pE+s1TXdob1mORJ4/UFdrifcy0=
+github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
-github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
-github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
+github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
 github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ=
 github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0=
 github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0=
@@ -109,8 +111,8 @@ golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
 golang.org/x/sync v0.14.0 h1:woo0S4Yywslg6hp4eUFjTVOyKt0RookbpAHG4c1HmhQ=
 golang.org/x/sync v0.14.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw=
+golang.org/x/sys v0.37.0 h1:fdNQudmxPjkdUTPnLn5mdQv7Zwvbvpaxqs831goi9kQ=
-golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
+golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
 golang.org/x/text v0.25.0 h1:qVyWApTSYLk/drJRO5mDlNYskwQznZmkpV2c8q9zls4=
 golang.org/x/text v0.25.0/go.mod h1:WEdwpYrmk1qmdHvhkSTNPm3app7v4rsT8F2UD6+VHIA=
 golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d h1:vU5i/LfpvrRCpgM/VPfJLg5KjxD3E+hfT1SH+d9zLwg=

internal/config/config.go

@@ -26,6 +26,10 @@ const (
 	// EnvironmentProd represents production environment.
 	EnvironmentProd = "prod"
 
+	// DefaultDataDir is where all SQLite databases live when DATA_DIR
+	// is unset. The same default applies in every environment.
+	DefaultDataDir = "/var/lib/webhooker"
+
 	// defaultPort is the default HTTP listen port.
 	defaultPort = 8080
 
@@ -159,6 +163,19 @@ func envString(key string) string {
 	return os.Getenv(key)
 }
 
+// DataDir resolves DATA_DIR, applying DefaultDataDir when it is unset
+// or empty. It is exported so that entry points which must act on the
+// data directory before the fx graph exists — taking the exclusive
+// directory lock, above all — resolve it exactly as Config does.
+func DataDir() string {
+	dir := envString("DATA_DIR")
+	if dir == "" {
+		return DefaultDataDir
+	}
+
+	return dir
+}
+
 // envBool returns the value of the named environment variable
 // parsed as a boolean. Returns defaultValue if not set. If the
 // variable is set but cannot be parsed, it returns a wrapped error
@@ -461,7 +478,7 @@ func loadFromEnv() (*Config, error) {
 	}
 
 	return &Config{
-		DataDir:                envString("DATA_DIR"),
+		DataDir:                DataDir(),
 		Debug:                  debug,
 		MaintenanceMode:        maintenanceMode,
 		Environment:            environment,
@@ -539,14 +556,6 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
 	s.log = log
 	s.params = &params
 
-	// Set default DataDir. All SQLite databases (main application
-	// DB and per-webhook event DBs) live here. The same default is
-	// used regardless of environment; override with DATA_DIR if
-	// needed.
-	if s.DataDir == "" {
-		s.DataDir = "/var/lib/webhooker"
-	}
-
 	if s.Debug {
 		params.Logger.EnableDebugLogging()
 	}

internal/config/config_test.go

@@ -424,6 +424,36 @@ func TestDefaultDataDir(t *testing.T) {
 	}
 }
 
+// TestDataDirHelper pins the exported resolver against the value
+// Config carries. The entry point takes the DATA_DIR lock through the
+// helper before the fx graph exists, so the two disagreeing would mean
+// locking one directory and writing to another.
+func TestDataDirHelper(t *testing.T) {
+	for _, set := range []string{"", "/tmp/webhooker-datadir-helper"} {
+		name := "set"
+		if set == "" {
+			name = "unset"
+		}
+
+		t.Run(name, func(t *testing.T) {
+			// Cannot use t.Parallel() here because t.Setenv
+			// is incompatible with parallel subtests.
+			if set == "" {
+				require.NoError(t, os.Unsetenv("DATA_DIR"))
+			} else {
+				t.Setenv("DATA_DIR", set)
+			}
+
+			expected := set
+			if expected == "" {
+				expected = config.DefaultDataDir
+			}
+
+			assert.Equal(t, expected, config.DataDir())
+		})
+	}
+}
+
 func TestReceiverRateLimit(t *testing.T) {
 	tests := []struct {
 		name        string

internal/datadir/lock.go

@@ -0,0 +1,126 @@
+// Package datadir guards exclusive access to the directory holding
+// every SQLite database webhooker writes.
+//
+// Two processes sharing a DATA_DIR each open the same per-webhook
+// event databases and each run delivery recovery over the same rows,
+// so every pending delivery goes out twice. SQLite's own locking does
+// not prevent that: both writers are serialised correctly and both
+// deliver. The only thing that prevents it is refusing to be the
+// second process.
+//
+// The lock lives here rather than in the server's fx graph so that any
+// entry point which touches DATA_DIR — the server, or a CLI
+// subcommand that must not operate on a live deployment's data — takes
+// it the same way.
+package datadir
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"github.com/gofrs/flock"
+)
+
+// LockFileName is the advisory lock file created inside DATA_DIR. Its
+// contents are never read: the lock is the flock(2) held on the open
+// descriptor, not the file's existence, so a leftover file from a
+// process that was killed with SIGKILL blocks nothing.
+const LockFileName = "webhooker.lock"
+
+// dirPerm is the mode Acquire creates DATA_DIR with. It matches what
+// internal/database uses, since whichever runs first creates it.
+const dirPerm = 0o750
+
+// ErrLocked reports that another live process holds the data
+// directory. Callers that need to know whether a deployment is running
+// — rather than merely failing to start — test for this with
+// errors.Is.
+var ErrLocked = errors.New(
+	"data directory is already in use by another instance",
+)
+
+// ErrNoDir reports that Acquire was given an empty directory.
+var ErrNoDir = errors.New("no data directory given")
+
+// Lock is a held exclusive advisory lock on a data directory. It is
+// valid only while the process that took it lives: the kernel drops it
+// when the descriptor closes, whether that is Release, a normal exit,
+// or a SIGKILL.
+type Lock struct {
+	dir  string
+	file *flock.Flock
+}
+
+// Acquire takes the exclusive advisory lock on dir, creating dir if it
+// does not exist. It never waits: if another process holds the lock it
+// returns an error wrapping ErrLocked and naming dir.
+//
+// The returned Lock must be held for as long as the caller intends to
+// use dir.
+func Acquire(dir string) (*Lock, error) {
+	if dir == "" {
+		return nil, ErrNoDir
+	}
+
+	err := os.MkdirAll(dir, dirPerm)
+	if err != nil {
+		return nil, fmt.Errorf(
+			"creating data directory %s: %w", dir, err,
+		)
+	}
+
+	path := filepath.Join(dir, LockFileName)
+	fl := flock.New(path)
+
+	held, err := fl.TryLock()
+	if err != nil {
+		return nil, fmt.Errorf(
+			"locking data directory %s: %w", dir, err,
+		)
+	}
+
+	if !held {
+		// A no-op on flock v0.13.0, which closes its own descriptor on
+		// a failed TryLock; kept so no version can leak one.
+		_ = fl.Close()
+
+		return nil, fmt.Errorf(
+			"%w: %s (%s). Only one webhooker may use a data "+
+				"directory: two both run delivery recovery over the "+
+				"same rows and both deliver",
+			ErrLocked, dir, path,
+		)
+	}
+
+	return &Lock{dir: dir, file: fl}, nil
+}
+
+// Dir returns the locked directory.
+func (l *Lock) Dir() string {
+	return l.dir
+}
+
+// Path returns the lock file backing the lock.
+func (l *Lock) Path() string {
+	return l.file.Path()
+}
+
+// Release drops the lock and closes the descriptor. It is safe to call
+// more than once.
+//
+// The lock file is deliberately left on disk. Unlinking it would let
+// the next process create and lock a fresh inode while a third still
+// holds the old one, which is the one outcome this package exists to
+// prevent.
+func (l *Lock) Release() error {
+	err := l.file.Unlock()
+	if err != nil {
+		return fmt.Errorf(
+			"releasing lock on data directory %s: %w", l.dir, err,
+		)
+	}
+
+	return nil
+}

internal/datadir/lock_test.go

@@ -0,0 +1,250 @@
+package datadir_test
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/datadir"
+)
+
+// holderEnv names the directory the re-executed test binary should
+// lock and hold. When it is unset the child test does nothing, so an
+// ordinary run is unaffected.
+const holderEnv = "WEBHOOKER_DATADIR_LOCK_HOLDER"
+
+// holderReadyPrefix labels the child's one-line report that it holds
+// the lock, so the parent can find it among the testing package's own
+// output on the same descriptor.
+const holderReadyPrefix = "DATADIR-LOCK-HELD "
+
+// holderReadyTimeout bounds the wait for the child to take the lock.
+// It only has to cover process start on a loaded shared host.
+const holderReadyTimeout = 60 * time.Second
+
+// holderHold is how long the child keeps the lock if nothing kills it.
+// A sleep rather than a bare block, so the runtime's deadlock detector
+// has a pending timer and the child cannot outlive a killed test run
+// by more than this.
+const holderHold = 10 * time.Minute
+
+// TestLockHolder is the child half of the two-process tests below. It
+// takes the lock on the directory named by holderEnv, reports the lock
+// file on standard output, and then holds it until it is killed.
+func TestLockHolder(t *testing.T) {
+	t.Parallel()
+
+	dir := os.Getenv(holderEnv)
+	if dir == "" {
+		return
+	}
+
+	lock, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+
+	// Written to the descriptor directly: the parent reads fd 1, not
+	// the testing package's buffered report.
+	_, err = fmt.Fprintf(
+		os.Stdout, "%s%s\n", holderReadyPrefix, lock.Path(),
+	)
+	require.NoError(t, err)
+
+	time.Sleep(holderHold)
+}
+
+// startHolder re-executes this test binary as a separate process that
+// takes and holds the lock on dir, and returns once that process
+// actually holds it. The child is killed when the test ends.
+func startHolder(t *testing.T, dir string) *exec.Cmd {
+	t.Helper()
+
+	//nolint:gosec // Re-executing this test binary, with a fixed arg.
+	cmd := exec.CommandContext(
+		t.Context(), os.Args[0], "-test.run", "^TestLockHolder$",
+	)
+
+	cmd.Env = append(os.Environ(), holderEnv+"="+dir)
+	cmd.Stderr = os.Stderr
+
+	stdout, err := cmd.StdoutPipe()
+	require.NoError(t, err)
+	require.NoError(t, cmd.Start())
+
+	t.Cleanup(func() {
+		_ = cmd.Process.Kill()
+		_ = cmd.Wait()
+	})
+
+	ready := make(chan string, 1)
+
+	go func() {
+		scanner := bufio.NewScanner(stdout)
+		for scanner.Scan() {
+			after, found := strings.CutPrefix(
+				scanner.Text(), holderReadyPrefix,
+			)
+			if found {
+				ready <- after
+
+				break
+			}
+		}
+
+		close(ready)
+
+		// Keep draining so the child never blocks on a full pipe.
+		_, _ = io.Copy(io.Discard, stdout)
+	}()
+
+	select {
+	case path, ok := <-ready:
+		require.True(
+			t, ok, "holder exited without taking the lock",
+		)
+		require.Equal(t, filepath.Join(dir, datadir.LockFileName), path)
+	case <-time.After(holderReadyTimeout):
+		t.Fatal("timed out waiting for the holder to take the lock")
+	}
+
+	return cmd
+}
+
+// TestSecondInstanceRefused is the regression test for the duplicate
+// delivery this package exists to prevent: a real second process
+// pointed at a data directory a live process already holds must be
+// refused, with an error that names the directory.
+func TestSecondInstanceRefused(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+
+	startHolder(t, dir)
+
+	lock, err := datadir.Acquire(dir)
+
+	require.Error(t, err, "the second instance took the lock too")
+	require.Nil(t, lock)
+	require.ErrorIs(
+		t, err, datadir.ErrLocked,
+		"the refusal must be distinguishable from any other failure",
+	)
+	assert.Contains(
+		t, err.Error(), dir,
+		"the refusal must name the directory it is about",
+	)
+}
+
+// TestRestartAfterHardKill is the other half of the regression: a
+// process killed with SIGKILL runs no cleanup and leaves its lock file
+// behind, and the next start must not be blocked by it. This is what a
+// pidfile would get wrong; the kernel drops a flock when the
+// descriptor closes, however the process died.
+func TestRestartAfterHardKill(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+
+	holder := startHolder(t, dir)
+
+	require.NoError(t, holder.Process.Kill())
+
+	// Wait for the kill to have actually happened. Re-acquiring while
+	// the corpse still holds a descriptor would be a race, and would
+	// make this test pass or fail on scheduling.
+	_ = holder.Wait()
+
+	require.FileExists(
+		t, filepath.Join(dir, datadir.LockFileName),
+		"the stale lock file is what must not block the restart",
+	)
+
+	lock, err := datadir.Acquire(dir)
+	require.NoError(
+		t, err, "a hard-killed instance must not block the next start",
+	)
+
+	require.NoError(t, lock.Release())
+}
+
+// TestSecondFdInSameProcessRefused pins the flock(2) property the
+// tests in cmd/webhooker rely on: descriptors are locked
+// independently, so a second acquisition is denied even when it comes
+// from the process that already holds the lock.
+func TestSecondFdInSameProcessRefused(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+
+	first, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+
+	defer func() { _ = first.Release() }()
+
+	_, err = datadir.Acquire(dir)
+	require.ErrorIs(t, err, datadir.ErrLocked)
+}
+
+// TestReleaseAllowsReacquire covers the clean-shutdown path: the lock
+// is released on exit, so a restart is not blocked by the previous
+// run.
+func TestReleaseAllowsReacquire(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+
+	first, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+	require.NoError(t, first.Release())
+
+	second, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+	require.NoError(t, second.Release())
+}
+
+// TestAcquireCreatesDataDir covers a first start against a DATA_DIR
+// that does not exist yet, which is the normal case for a fresh
+// deployment: the lock is taken before anything else creates it.
+func TestAcquireCreatesDataDir(t *testing.T) {
+	t.Parallel()
+
+	dir := filepath.Join(t.TempDir(), "nested", "data")
+
+	lock, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+
+	defer func() { _ = lock.Release() }()
+
+	assert.Equal(t, dir, lock.Dir())
+	assert.FileExists(t, filepath.Join(dir, datadir.LockFileName))
+}
+
+// TestAcquireEmptyDir rejects an empty directory rather than locking
+// the process's working directory.
+func TestAcquireEmptyDir(t *testing.T) {
+	t.Parallel()
+
+	_, err := datadir.Acquire("")
+	require.ErrorIs(t, err, datadir.ErrNoDir)
+}
+
+// TestAcquireUnusableDir reports an unusable DATA_DIR clearly, naming
+// it, instead of failing later and deeper.
+func TestAcquireUnusableDir(t *testing.T) {
+	t.Parallel()
+
+	file := filepath.Join(t.TempDir(), "not-a-directory")
+	require.NoError(t, os.WriteFile(file, nil, 0o600))
+
+	_, err := datadir.Acquire(file)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), file)
+}