From 7da2942ca6481995059dc194ea7ed1bfd1b91250 Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 9 Oct 2025 10:38:01 +0200 Subject: [PATCH] fix: correctly set database path on windows (#1616) --- config-raw.json | 2 +- pkg/db/db.go | 159 +++++++++++++++++++- pkg/db/db_path_test.go | 332 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 488 insertions(+), 5 deletions(-) create mode 100644 pkg/db/db_path_test.go diff --git a/config-raw.json b/config-raw.json index 332551252..fad9a59e8 100644 --- a/config-raw.json +++ b/config-raw.json @@ -196,7 +196,7 @@ { "key": "path", "default_value": "./vikunja.db", - "comment": "When using sqlite, this is the path where to store the data" + "comment": "When using sqlite, this is the path where to store the database file. Can be an absolute path or relative path. \u003cbr/\u003e\nRelative paths are resolved as follows: \u003cbr/\u003e\n- If `service.rootpath` is explicitly configured (differs from the binary location), the database path is resolved relative to that directory. \u003cbr/\u003e\n- Otherwise, relative paths are resolved to a platform-specific user data directory to prevent database files from being created in system directories (like `C:\\Windows\\System32` on Windows when running as a service): \u003cbr/\u003e\n - **Windows**: `%LOCALAPPDATA%\\Vikunja` (e.g., `C:\\Users\\username\\AppData\\Local\\Vikunja`) \u003cbr/\u003e\n - **macOS**: `~/Library/Application Support/Vikunja` \u003cbr/\u003e\n - **Linux**: `$XDG_DATA_HOME/vikunja` or `~/.local/share/vikunja` \u003cbr/\u003e\n**Recommendation**: Use an absolute path for production deployments, especially when running Vikunja as a Windows service, to have full control over the database location." }, { "key": "maxopenconnections", diff --git a/pkg/db/db.go b/pkg/db/db.go index 40a908bf5..01d567b1d 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -21,6 +21,7 @@ import ( "net/url" "os" "path/filepath" + "runtime" "strconv" "strings" "time" @@ -187,14 +188,29 @@ func initPostgresEngine() (engine *xorm.Engine, err error) { func initSqliteEngine() (engine *xorm.Engine, err error) { path := config.DatabasePath.GetString() - if path != "memory" && !filepath.IsAbs(path) { - path = filepath.Join(config.ServiceRootpath.GetString(), path) - } - if path == "memory" { return xorm.NewEngine("sqlite3", "file::memory:?cache=shared") } + // Resolve relative paths to a safe location instead of the current working directory + if !filepath.IsAbs(path) { + path = resolveDatabasePath(path) + } + + // Convert to absolute path for logging + path, err = filepath.Abs(path) + if err != nil { + return nil, fmt.Errorf("could not resolve database path to absolute path: %w", err) + } + + // Log the resolved database path + log.Infof("Using SQLite database at: %s", path) + + // Warn if the database is in a potentially problematic location + if isSystemDirectory(path) { + log.Warningf("Database path (%s) appears to be in a system directory. This may cause issues. Please use an absolute path or configure the database path to a user data directory.", path) + } + // Try opening the db file to return a better error message if that does not work var exists = true if _, err := os.Stat(path); err != nil { @@ -213,6 +229,141 @@ func initSqliteEngine() (engine *xorm.Engine, err error) { return xorm.NewEngine("sqlite3", path) } +// resolveDatabasePath resolves a relative database path to an appropriate location +// based on platform-specific conventions. This prevents SQLite databases from being +// created in system directories (like C:\Windows\System32 on Windows) when Vikunja +// runs as a service. +func resolveDatabasePath(relativePath string) string { + // First, check if a rootpath is explicitly configured and use that + rootPath := config.ServiceRootpath.GetString() + + // Determine if rootpath appears to be a user-configured value or a default + // If it looks like it was explicitly configured (not just the binary location), + // use it for backward compatibility + execPath, err := os.Executable() + if err == nil && rootPath != filepath.Dir(execPath) { + // rootpath was explicitly configured, use it + return filepath.Join(rootPath, relativePath) + } + + // Otherwise, use a platform-appropriate user data directory + dataDir, err := getUserDataDir() + if err != nil { + // Fall back to rootpath if we can't determine user data dir + log.Warningf("Could not determine user data directory, falling back to rootpath: %v", err) + return filepath.Join(rootPath, relativePath) + } + + return filepath.Join(dataDir, relativePath) +} + +// getUserDataDir returns the platform-appropriate directory for application data +func getUserDataDir() (string, error) { + var dataDir string + + switch runtime.GOOS { + case "windows": + // On Windows, use %LOCALAPPDATA%\Vikunja + localAppData := os.Getenv("LOCALAPPDATA") + if localAppData == "" { + // Fallback to %USERPROFILE%\AppData\Local if LOCALAPPDATA is not set + userProfile := os.Getenv("USERPROFILE") + if userProfile == "" { + return "", fmt.Errorf("neither LOCALAPPDATA nor USERPROFILE environment variables are set") + } + localAppData = filepath.Join(userProfile, "AppData", "Local") + } + dataDir = filepath.Join(localAppData, "Vikunja") + case "darwin": + // On macOS, use ~/Library/Application Support/Vikunja + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + dataDir = filepath.Join(home, "Library", "Application Support", "Vikunja") + default: + // On Linux and other Unix-like systems, use XDG_DATA_HOME or ~/.local/share/vikunja + xdgDataHome := os.Getenv("XDG_DATA_HOME") + if xdgDataHome != "" { + dataDir = filepath.Join(xdgDataHome, "vikunja") + } else { + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + dataDir = filepath.Join(home, ".local", "share", "vikunja") + } + } + + // Ensure the directory exists + if err := os.MkdirAll(dataDir, 0o700); err != nil { + return "", fmt.Errorf("could not create data directory %s: %w", dataDir, err) + } + + return dataDir, nil +} + +// isSystemDirectory checks if a path appears to be in a system directory +// where users should not typically store application data +func isSystemDirectory(path string) bool { + // Clean and normalize the path + path = filepath.Clean(path) + lowerPath := strings.ToLower(path) + + // Windows system directories + if runtime.GOOS == "windows" { + // Convert to absolute path if possible for more accurate checking + absPath := lowerPath + if abs, err := filepath.Abs(path); err == nil { + absPath = strings.ToLower(filepath.Clean(abs)) + } + + // Check common Windows system directories using prefix matching + // This prevents false positives like C:\myapp\windows\data + windowsSystemPrefixes := []string{ + "c:\\windows\\system32", + "c:\\windows\\syswow64", + "c:\\windows\\winsxs", + "c:\\windows\\servicing", + } + + for _, prefix := range windowsSystemPrefixes { + if strings.HasPrefix(absPath, prefix) { + return true + } + } + + // Also check for direct C:\Windows (not subdirectories like C:\myapp\windows) + // by ensuring it starts with the drive and windows directory + if absPath == "c:\\windows" || strings.HasPrefix(absPath, "c:\\windows\\") { + // Exclude some safe subdirectories under C:\Windows + safeDirs := []string{ + "c:\\windows\\temp", + } + for _, safeDir := range safeDirs { + if strings.HasPrefix(absPath, safeDir) { + return false + } + } + return true + } + } + + // Unix-like system directories - use prefix matching + systemDirs := []string{ + "/bin", "/sbin", "/usr/bin", "/usr/sbin", + "/etc", "/sys", "/proc", "/dev", + } + for _, sysDir := range systemDirs { + // Ensure we match exact directory boundaries + if lowerPath == sysDir || strings.HasPrefix(lowerPath, sysDir+"/") { + return true + } + } + + return false +} + // WipeEverything wipes all tables and their data. Use with caution... func WipeEverything() error { diff --git a/pkg/db/db_path_test.go b/pkg/db/db_path_test.go new file mode 100644 index 000000000..10f95cc05 --- /dev/null +++ b/pkg/db/db_path_test.go @@ -0,0 +1,332 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package db + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "code.vikunja.io/api/pkg/config" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResolveDatabasePath(t *testing.T) { + // Save original config values + originalRootpath := config.ServiceRootpath.GetString() + defer config.ServiceRootpath.Set(originalRootpath) + + t.Run("with explicitly configured rootpath", func(t *testing.T) { + // Set a rootpath that is different from the executable location + config.ServiceRootpath.Set("/custom/path") + + result := resolveDatabasePath("vikunja.db") + expected := filepath.Join("/custom/path", "vikunja.db") + + assert.Equal(t, expected, result) + }) + + t.Run("with default rootpath uses user data directory", func(t *testing.T) { + // Get the actual executable path and set rootpath to it + execPath, err := os.Executable() + require.NoError(t, err) + config.ServiceRootpath.Set(filepath.Dir(execPath)) + + result := resolveDatabasePath("vikunja.db") + + // Result should contain the platform-specific user data directory + // and not be in the executable directory + assert.NotEqual(t, filepath.Join(filepath.Dir(execPath), "vikunja.db"), result) + assert.Contains(t, result, "vikunja.db") + + // Verify it's using a platform-appropriate path + switch runtime.GOOS { + case "windows": + // Should be in %LOCALAPPDATA%\Vikunja or %USERPROFILE%\AppData\Local\Vikunja + assert.Contains(t, result, "Vikunja") + case "darwin": + // Should be in ~/Library/Application Support/Vikunja + assert.Contains(t, result, "Library") + assert.Contains(t, result, "Application Support") + default: + // Should be in ~/.local/share/vikunja or $XDG_DATA_HOME/vikunja + assert.NotEqual(t, + filepath.Dir(result), + filepath.Dir(execPath), + "Database should not be in executable directory", + ) + } + }) + + t.Run("with subdirectory path", func(t *testing.T) { + config.ServiceRootpath.Set("/custom/path") + + result := resolveDatabasePath("data/vikunja.db") + expected := filepath.Join("/custom/path", "data", "vikunja.db") + + assert.Equal(t, expected, result) + }) +} + +func TestGetUserDataDir(t *testing.T) { + + test := func() string { + dataDir, err := getUserDataDir() + require.NoError(t, err) + assert.NotEmpty(t, dataDir) + + // Verify the directory was created + info, err := os.Stat(dataDir) + require.NoError(t, err) + assert.True(t, info.IsDir()) + + return dataDir + } + + // Verify platform-specific paths + switch runtime.GOOS { + case "windows": + dataDir := test() + assert.Contains(t, dataDir, "Vikunja") + case "darwin": + dataDir := test() + assert.Contains(t, dataDir, "Library") + assert.Contains(t, dataDir, "Application Support") + assert.Contains(t, dataDir, "Vikunja") + default: + originalXDGDataHome := os.Getenv("XDG_DATA_HOME") + defer func() { + if originalXDGDataHome != "" { + os.Setenv("XDG_DATA_HOME", originalXDGDataHome) + } else { + os.Unsetenv("XDG_DATA_HOME") + } + }() + + t.Run("with XDG_DATA_HOME", func(t *testing.T) { + os.Setenv("XDG_DATA_HOME", "/tmp") + dataDir := test() + assert.Contains(t, dataDir, filepath.Join("/tmp", "vikunja")) + }) + + t.Run("without XDG_DATA_HOME", func(t *testing.T) { + os.Unsetenv("XDG_DATA_HOME") + dataDir := test() + assert.Contains(t, dataDir, "vikunja") + }) + } +} + +func TestIsSystemDirectory(t *testing.T) { + tests := []struct { + name string + path string + expected bool + }{ + // Windows system directories + { + name: "Windows System32", + path: "C:\\Windows\\System32\\vikunja.db", + expected: runtime.GOOS == "windows", + }, + { + name: "Windows SysWOW64", + path: "C:\\Windows\\SysWOW64\\vikunja.db", + expected: runtime.GOOS == "windows", + }, + { + name: "Windows root", + path: "C:\\Windows\\vikunja.db", + expected: runtime.GOOS == "windows", + }, + { + name: "Windows System32 lowercase", + path: "c:\\windows\\system32\\vikunja.db", + expected: runtime.GOOS == "windows", + }, + // Unix-like system directories + { + name: "/bin", + path: "/bin/vikunja.db", + expected: runtime.GOOS != "windows", + }, + { + name: "/sbin", + path: "/sbin/vikunja.db", + expected: runtime.GOOS != "windows", + }, + { + name: "/usr/bin", + path: "/usr/bin/vikunja.db", + expected: runtime.GOOS != "windows", + }, + { + name: "/etc", + path: "/etc/vikunja.db", + expected: runtime.GOOS != "windows", + }, + // Non-system directories + { + name: "user home directory (Unix)", + path: "/home/user/vikunja.db", + expected: false, + }, + { + name: "user profile directory (Windows)", + path: "C:\\Users\\user\\vikunja.db", + expected: false, + }, + { + name: "custom directory", + path: "/opt/vikunja/vikunja.db", + expected: false, + }, + { + name: "relative path", + path: "./vikunja.db", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isSystemDirectory(tt.path) + assert.Equal(t, tt.expected, result, + "Expected isSystemDirectory(%s) to be %v on %s", + tt.path, tt.expected, runtime.GOOS) + }) + } +} + +func TestIsSystemDirectory_EdgeCases(t *testing.T) { + if runtime.GOOS == "windows" { + t.Run("false positives - paths containing 'windows' but not system directories", func(t *testing.T) { + tests := []struct { + name string + path string + }{ + { + name: "custom app with windows in path", + path: "C:\\myapp\\windows\\data\\vikunja.db", + }, + { + name: "windows directory on non-C drive", + path: "D:\\windows\\vikunja.db", + }, + { + name: "user directory named windows", + path: "C:\\Users\\windows\\vikunja.db", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.False(t, isSystemDirectory(tt.path)) + }) + } + }) + + t.Run("safe Windows subdirectories", func(t *testing.T) { + assert.False(t, isSystemDirectory("C:\\Windows\\Temp\\vikunja.db")) + }) + + t.Run("actual Windows system directories", func(t *testing.T) { + tests := []struct { + name string + path string + }{ + { + name: "Windows root", + path: "C:\\Windows\\vikunja.db", + }, + { + name: "Windows root lowercase", + path: "c:\\windows\\vikunja.db", + }, + { + name: "System32", + path: "C:\\Windows\\System32\\vikunja.db", + }, + { + name: "System32 uppercase", + path: "C:\\WINDOWS\\SYSTEM32\\vikunja.db", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.True(t, isSystemDirectory(tt.path)) + }) + } + }) + } else { + t.Run("false positives - paths containing system dir names", func(t *testing.T) { + tests := []struct { + name string + path string + }{ + { + name: "/home/bin not same as /bin", + path: "/home/bin/vikunja.db", + }, + { + name: "/opt/sbin not same as /sbin", + path: "/opt/sbin/vikunja.db", + }, + { + name: "/usr/local/bin is safe", + path: "/usr/local/bin/vikunja.db", + }, + { + name: "/binaries not same as /bin", + path: "/binaries/vikunja.db", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.False(t, isSystemDirectory(tt.path)) + }) + } + }) + + t.Run("actual Unix system directories", func(t *testing.T) { + tests := []struct { + name string + path string + }{ + { + name: "/bin", + path: "/bin/vikunja.db", + }, + { + name: "/etc", + path: "/etc/vikunja.db", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.True(t, isSystemDirectory(tt.path)) + }) + } + }) + } +}