From 4c7272170ac01b994b9b7421b3812c3afcbfc3b0 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Sep 2026 07:48:44 +0000 Subject: [PATCH] Consolidate the data directory mode into one owner (closes #288) internal/datadir now owns the DATA_DIR mode as an exported constant, DirPerm; internal/database consumes it in both places that create the directory rather than keeping its own copy. datadir is the owner because guarding and creating DATA_DIR is its whole purpose, and the dependency runs the right way: datadir imports only the standard library and the flock package, so database importing datadir adds no cycle. A comment on DirPerm names it as the single source, chosen over a source-scanning test because it survives refactoring and builds no machinery that inspects the tree. The mode value is unchanged. Model: opus-4-8 --- internal/database/database.go | 6 ++++-- internal/database/webhook_db_manager.go | 6 ++++-- internal/datadir/lock.go | 10 ++++++---- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/internal/database/database.go b/internal/database/database.go index f2880e8..ba28bae 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -17,12 +17,12 @@ import ( "gorm.io/gorm" "sneak.berlin/go/webhooker/internal/banner" "sneak.berlin/go/webhooker/internal/config" + "sneak.berlin/go/webhooker/internal/datadir" "sneak.berlin/go/webhooker/internal/gormlog" "sneak.berlin/go/webhooker/internal/logger" ) const ( - dataDirPerm = 0750 randomPasswordLen = 16 sessionKeyLen = 32 ) @@ -185,7 +185,9 @@ func (d *Database) connect() error { // caller's decision. func (d *Database) connectTo(dataDir string) error { // Ensure the data directory exists before opening the database. - err := os.MkdirAll(dataDir, dataDirPerm) + // datadir.DirPerm is the single source of the directory mode; this + // package creates the directory too, since either may run first. + err := os.MkdirAll(dataDir, datadir.DirPerm) if err != nil { return fmt.Errorf( "creating data directory %s: %w", diff --git a/internal/database/webhook_db_manager.go b/internal/database/webhook_db_manager.go index 81ca427..7da8169 100644 --- a/internal/database/webhook_db_manager.go +++ b/internal/database/webhook_db_manager.go @@ -13,6 +13,7 @@ import ( "gorm.io/driver/sqlite" "gorm.io/gorm" "sneak.berlin/go/webhooker/internal/config" + "sneak.berlin/go/webhooker/internal/datadir" "sneak.berlin/go/webhooker/internal/gormlog" "sneak.berlin/go/webhooker/internal/logger" ) @@ -53,8 +54,9 @@ func NewWebhookDBManager( log: params.Logger.Get(), } - // Create data directory if it doesn't exist - err := os.MkdirAll(m.dataDir, dataDirPerm) + // Create data directory if it doesn't exist. datadir.DirPerm is the + // single source of the directory mode; either package may run first. + err := os.MkdirAll(m.dataDir, datadir.DirPerm) if err != nil { return nil, fmt.Errorf( "creating data directory %s: %w", diff --git a/internal/datadir/lock.go b/internal/datadir/lock.go index a50929c..bb605cd 100644 --- a/internal/datadir/lock.go +++ b/internal/datadir/lock.go @@ -29,9 +29,11 @@ import ( // 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 +// DirPerm is the mode DATA_DIR is created with. It is the single +// source of that mode: internal/database consumes it rather than +// keeping its own copy, so the two packages that both create the +// directory cannot drift into disagreeing about its permissions. +const DirPerm = 0o750 // ErrLocked reports that another live process holds the data // directory. Callers that need to know whether a deployment is running @@ -64,7 +66,7 @@ func Acquire(dir string) (*Lock, error) { return nil, ErrNoDir } - err := os.MkdirAll(dir, dirPerm) + err := os.MkdirAll(dir, DirPerm) if err != nil { return nil, fmt.Errorf( "creating data directory %s: %w", dir, err,