security: fix data loss on container restart due to relative paths
Root cause: .env.example used relative paths (./data/...) which resolve to /app/data/ inside the container instead of the persistent volume at /data/. This caused the database to be recreated on every container restart, resetting the admin password to a new initial value. Fixes: - .env.example: comment out path settings with clear warning about relative paths; Dockerfile already provides correct absolute defaults - auth: add initial_setup_complete flag in settings table as defence-in-depth so EnsureAdmin never re-creates an admin after the initial setup, even if the users table is unexpectedly empty - main: add validateDataPaths() startup check that warns when relative container paths are detected (potential data-loss misconfiguration) - auth_test: extend TestEnsureAdmin to verify the flag prevents admin re-creation after user deletion
This commit is contained in:
+9
-4
@@ -20,10 +20,15 @@ KEYWARDEN_ENCRYPTION_KEY=change-me-encryption-key-32chars
|
||||
KEYWARDEN_LOG_LEVEL=INFO
|
||||
|
||||
# --- Paths (optional, Docker defaults are usually fine) ---
|
||||
KEYWARDEN_DB_PATH=./data/keywarden.db
|
||||
KEYWARDEN_DATA_DIR=./data
|
||||
KEYWARDEN_KEYS_DIR=./data/keys
|
||||
KEYWARDEN_MASTER_DIR=./data/master
|
||||
# IMPORTANT: These paths refer to locations INSIDE the Docker container.
|
||||
# The Dockerfile already sets correct defaults (/data/...). Only override
|
||||
# if you know what you are doing. Do NOT use relative paths (./data/...)
|
||||
# – they resolve to /app/data/ inside the container and bypass the
|
||||
# persistent volume mount at /data, causing DATA LOSS on restart.
|
||||
# KEYWARDEN_DB_PATH=/data/keywarden.db
|
||||
# KEYWARDEN_DATA_DIR=/data
|
||||
# KEYWARDEN_KEYS_DIR=/data/keys
|
||||
# KEYWARDEN_MASTER_DIR=/data/master
|
||||
|
||||
# --- Security / Hardening (optional) ---
|
||||
# Public URL used for email links and cookie config.
|
||||
|
||||
@@ -7,6 +7,8 @@ package main
|
||||
import (
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
||||
"git.techniverse.net/scriptos/keywarden/internal/audit"
|
||||
"git.techniverse.net/scriptos/keywarden/internal/auth"
|
||||
@@ -34,6 +36,10 @@ func main() {
|
||||
logging.Info("🔑 Keywarden - Centralized SSH Key Management and Deployment")
|
||||
logging.Info(" https://git.techniverse.net/scriptos/keywarden")
|
||||
|
||||
// Validate data paths – relative paths inside a container bypass the
|
||||
// persistent volume mount and lead to silent data loss on restart.
|
||||
validateDataPaths(cfg)
|
||||
|
||||
// Ensure data directories exist
|
||||
for _, dir := range []string{cfg.DataDir, cfg.KeysDir, cfg.MasterDir} {
|
||||
if err := os.MkdirAll(dir, 0700); err != nil {
|
||||
@@ -152,3 +158,33 @@ func getEnvWithLegacy(primary, legacy, fallback string) string {
|
||||
}
|
||||
return fallback
|
||||
}
|
||||
|
||||
// validateDataPaths checks for a common misconfiguration: relative paths
|
||||
// (e.g. ./data/...) that resolve to the container's working directory instead
|
||||
// of the persistent volume mount. This would cause silent data loss on every
|
||||
// container restart.
|
||||
func validateDataPaths(cfg *config.Config) {
|
||||
paths := map[string]string{
|
||||
"KEYWARDEN_DB_PATH": cfg.DBPath,
|
||||
"KEYWARDEN_DATA_DIR": cfg.DataDir,
|
||||
"KEYWARDEN_KEYS_DIR": cfg.KeysDir,
|
||||
"KEYWARDEN_MASTER_DIR": cfg.MasterDir,
|
||||
}
|
||||
|
||||
for envVar, p := range paths {
|
||||
if p == "" {
|
||||
continue
|
||||
}
|
||||
abs, err := filepath.Abs(p)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
// Detect relative paths that resolve outside /data (the expected volume).
|
||||
if !filepath.IsAbs(p) || (!strings.HasPrefix(abs, "/data") && !strings.HasPrefix(abs, `\data`)) {
|
||||
// Only warn – don't block startup for non-Docker environments.
|
||||
if strings.HasPrefix(p, "./") || strings.HasPrefix(p, "../") || (!filepath.IsAbs(p) && p != "") {
|
||||
logging.Warn("⚠ %s is a relative path (%s → %s). Inside a Docker container this may bypass the persistent volume and cause DATA LOSS on restart. Use an absolute path like /data/... instead.", envVar, p, abs)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -156,12 +156,23 @@ func (s *Service) HasUsers() (bool, error) {
|
||||
|
||||
// EnsureAdmin creates a default owner user if no users exist.
|
||||
// It auto-generates a secure password and returns (created, generatedPassword, error).
|
||||
// A persistent flag ("initial_setup_complete") is stored in the settings table
|
||||
// so that an admin account is never re-created after the initial setup, even
|
||||
// if the users table is unexpectedly empty (e.g. due to a misconfigured volume).
|
||||
func (s *Service) EnsureAdmin(username, email string) (bool, string, error) {
|
||||
// Defence-in-depth: if the initial setup was already completed once,
|
||||
// never auto-create another admin – even when the users table is empty.
|
||||
if s.isInitialSetupComplete() {
|
||||
return false, "", nil
|
||||
}
|
||||
|
||||
hasUsers, err := s.HasUsers()
|
||||
if err != nil {
|
||||
return false, "", err
|
||||
}
|
||||
if hasUsers {
|
||||
// Users exist but no flag yet (upgrade path) – set the flag now.
|
||||
s.markInitialSetupComplete()
|
||||
return false, "", nil
|
||||
}
|
||||
|
||||
@@ -183,9 +194,28 @@ func (s *Service) EnsureAdmin(username, email string) (bool, string, error) {
|
||||
if err != nil {
|
||||
return false, "", err
|
||||
}
|
||||
|
||||
// Mark initial setup as complete so the password is never regenerated.
|
||||
s.markInitialSetupComplete()
|
||||
|
||||
return true, password, nil
|
||||
}
|
||||
|
||||
// isInitialSetupComplete checks whether the initial admin setup has already
|
||||
// been performed by looking for a flag in the settings table.
|
||||
func (s *Service) isInitialSetupComplete() bool {
|
||||
var val string
|
||||
err := s.db.QueryRow(`SELECT value FROM settings WHERE key = 'initial_setup_complete'`).Scan(&val)
|
||||
return err == nil && val == "true"
|
||||
}
|
||||
|
||||
// markInitialSetupComplete persists the initial-setup flag in the settings table.
|
||||
func (s *Service) markInitialSetupComplete() {
|
||||
s.db.Exec(
|
||||
`INSERT OR REPLACE INTO settings (key, value, updated_at) VALUES ('initial_setup_complete', 'true', CURRENT_TIMESTAMP)`,
|
||||
)
|
||||
}
|
||||
|
||||
// generateSecurePassword creates a cryptographically secure random password
|
||||
func generateSecurePassword(length int) (string, error) {
|
||||
const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
|
||||
|
||||
@@ -236,7 +236,7 @@ func TestEnsureAdmin(t *testing.T) {
|
||||
t.Fatalf("Expected owner role, got %q", user.Role)
|
||||
}
|
||||
|
||||
// Second call should be no-op
|
||||
// Second call should be no-op (users exist)
|
||||
created2, _, err := svc.EnsureAdmin("admin2", "admin2@test.com")
|
||||
if err != nil {
|
||||
t.Fatalf("Second EnsureAdmin should not fail: %v", err)
|
||||
@@ -250,6 +250,25 @@ func TestEnsureAdmin(t *testing.T) {
|
||||
if err != ErrInvalidCredentials {
|
||||
t.Fatalf("admin2 should not have been created")
|
||||
}
|
||||
|
||||
// initial_setup_complete flag should be set
|
||||
if !svc.isInitialSetupComplete() {
|
||||
t.Fatal("Expected initial_setup_complete flag to be set after EnsureAdmin")
|
||||
}
|
||||
|
||||
// Even if all users are deleted, EnsureAdmin must NOT create a new admin
|
||||
// because the initial_setup_complete flag is set (defence-in-depth).
|
||||
_, err = db.Exec(`DELETE FROM users`)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to delete all users: %v", err)
|
||||
}
|
||||
created3, _, err := svc.EnsureAdmin("admin3", "admin3@test.com")
|
||||
if err != nil {
|
||||
t.Fatalf("Third EnsureAdmin should not fail: %v", err)
|
||||
}
|
||||
if created3 {
|
||||
t.Fatal("EnsureAdmin must not create a user when initial_setup_complete flag is set")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetAllUsers(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user