Skip to content

Commit 0064730

Browse files
committed
fix(db): restore base directory-creation semantics
Creating the parent of every resolved path was too broad. An absolute or rootpath-relative database.path with a missing parent used to fail loudly and create nothing; it silently created a 0700 tree and an empty database instead — so a typo'd or unmounted path looked like data loss. The scratch Docker image sets VIKUNJA_DATABASE_PATH=/db/vikunja.db with no /db, so an unmounted volume would have written to the container layer. It also killed the rootpath fallback for an unwritable user data directory, turning a warn-and-continue into a refusal to start — upgrade-breaking for containers whose service user has an unwritable HOME. Creation moves back inside the injected getter: resolution passes existingUserDataDir (stat only), the engine passes createdUserDataDir. Both fall back to rootpath on failure, so they agree once the directory exists.
1 parent f4272c8 commit 0064730

2 files changed

Lines changed: 163 additions & 66 deletions

File tree

pkg/db/db.go

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -248,10 +248,11 @@ type DatabasePathConfig struct {
248248
// 3. If ConfiguredPath is relative:
249249
// a. If RootPath differs from ExecutablePath (explicitly configured),
250250
// joins with RootPath
251-
// b. Otherwise, joins with platform-specific user data directory
251+
// b. Otherwise, joins with the user data directory, falling back to RootPath
252+
// when userDataDir reports the directory as unavailable
252253
//
253-
// Resolution is pure: it never creates anything, so both callers agree on the
254-
// path. The userDataDir parameter allows injecting a mock for testing.
254+
// Directory creation only ever happens inside userDataDir, so an operator-configured
255+
// path never gets its parents created behind their back.
255256
func resolveDatabasePath(cfg DatabasePathConfig, userDataDir func() (string, error)) (string, error) {
256257
if cfg.ConfiguredPath == DatabasePathMemory {
257258
return DatabasePathMemory, nil
@@ -296,27 +297,13 @@ func databasePathConfig() DatabasePathConfig {
296297
// or DatabasePathMemory for the ephemeral database. Only meaningful when
297298
// database.type is sqlite. It creates nothing — see ensureDatabasePath.
298299
func ResolvedDatabasePath() (string, error) {
299-
return resolveDatabasePath(databasePathConfig(), resolveUserDataDir)
300+
return resolveDatabasePath(databasePathConfig(), existingUserDataDir)
300301
}
301302

302-
// ensureDatabasePath returns the same path as ResolvedDatabasePath, creating its
303-
// parent directory.
303+
// ensureDatabasePath returns the path ResolvedDatabasePath reports once the user data
304+
// directory exists, creating that directory if it is the one selected.
304305
func ensureDatabasePath() (string, error) {
305-
path, err := ResolvedDatabasePath()
306-
if err != nil {
307-
return "", err
308-
}
309-
310-
if path == DatabasePathMemory {
311-
return path, nil
312-
}
313-
314-
dir := filepath.Dir(path)
315-
if err := os.MkdirAll(dir, 0o700); err != nil { // #nosec G703 -- dir is from config or XDG standard paths
316-
return "", fmt.Errorf("could not create database directory %s: %w", dir, err)
317-
}
318-
319-
return path, nil
306+
return resolveDatabasePath(databasePathConfig(), createdUserDataDir)
320307
}
321308

322309
func initSqliteEngine() (engine *xorm.Engine, err error) {
@@ -375,6 +362,41 @@ func initSqliteEngine() (engine *xorm.Engine, err error) {
375362
return
376363
}
377364

365+
// existingUserDataDir returns the user data directory only when it is already there.
366+
// Treating an absent directory as unavailable keeps resolution free of side effects
367+
// while still reporting the path ensureDatabasePath uses, once it has run.
368+
func existingUserDataDir() (string, error) {
369+
dir, err := resolveUserDataDir()
370+
if err != nil {
371+
return "", err
372+
}
373+
374+
info, err := os.Stat(dir)
375+
if err != nil {
376+
return "", fmt.Errorf("could not stat data directory %s: %w", dir, err)
377+
}
378+
if !info.IsDir() {
379+
return "", fmt.Errorf("data directory %s is not a directory", dir)
380+
}
381+
382+
return dir, nil
383+
}
384+
385+
// createdUserDataDir creates the user data directory. Creation lives here so a failure
386+
// surfaces as the rootpath fallback instead of a hard startup error.
387+
func createdUserDataDir() (string, error) {
388+
dir, err := resolveUserDataDir()
389+
if err != nil {
390+
return "", err
391+
}
392+
393+
if err := os.MkdirAll(dir, 0o700); err != nil { // #nosec G703 -- dir is from XDG standard paths
394+
return "", fmt.Errorf("could not create data directory %s: %w", dir, err)
395+
}
396+
397+
return dir, nil
398+
}
399+
378400
func resolveUserDataDir() (string, error) {
379401
var dataDir string
380402

0 commit comments

Comments
 (0)