From b769664b23f453058fbb5320e85a9b9d7efb5dc0 Mon Sep 17 00:00:00 2001 From: Prakersh Maheshwari Date: Wed, 11 Mar 2026 19:14:46 +0530 Subject: [PATCH] fix(store): add DB path preflight for Docker permission errors Fail fast with actionable diagnostics when the SQLite DB path is not writable, including existing read-only database files. Preserve valid SQLite file: URIs and memory DSNs, and update Docker bind-mount guidance to use recursive chown. Follow-up to #22. --- README.md | 2 +- docker-compose.yml | 2 +- internal/store/store.go | 88 +++++++++++++++++++++++++++++ internal/store/store_test.go | 106 +++++++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d488217..35d7073 100644 --- a/README.md +++ b/README.md @@ -469,7 +469,7 @@ The `docker-compose.yml` includes memory limits (64M limit, 32M reservation), lo ### Troubleshooting -**Database errors:** Pre-create bind mount directories with `sudo chown 65532:65532` or use named volumes. +**Database path is not writable:** If startup shows `database path is not writable`, fix bind mount ownership recursively with `sudo chown -R 65532:65532 ./onwatch-data` or use named volumes. **Container won't start:** Check `docker-compose logs -f`; verify API keys in `.env` and port 9211 availability. **Debugging:** The distroless image has no shell - use a sidecar: `docker run -it --rm --pid=container:onwatch --net=container:onwatch nicolaka/netshoot bash` diff --git a/docker-compose.yml b/docker-compose.yml index c48b87d..efc56b5 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,7 +14,7 @@ services: - "${ONWATCH_PORT:-9211}:9211" # Volume mount for persistent SQLite database - # Bind mount — pre-create with: mkdir -p ./onwatch-data + # Bind mount - pre-create with: mkdir -p ./onwatch-data && chown -R 65532:65532 ./onwatch-data volumes: - ./onwatch-data:/data diff --git a/internal/store/store.go b/internal/store/store.go index 40f7c8f..e28a16a 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -5,6 +5,9 @@ import ( "encoding/base64" "errors" "fmt" + "net/url" + "os" + "path/filepath" "strings" "time" @@ -65,8 +68,93 @@ type CrossQuotaEntry struct { Delta float64 // Percent - StartPercent } +func preflightDatabasePath(dbPath string) error { + trimmed := strings.TrimSpace(dbPath) + if trimmed == "" { + return fmt.Errorf("failed to open database: empty database path") + } + + lower := strings.ToLower(trimmed) + if trimmed == ":memory:" || strings.HasPrefix(lower, "file::memory:") { + return nil + } + + resolvedPath := trimmed + if strings.HasPrefix(lower, "file:") { + parsed, parseErr := url.Parse(trimmed) + if parseErr == nil { + if strings.EqualFold(parsed.Query().Get("mode"), "memory") { + return nil + } + switch { + case parsed.Path != "": + resolvedPath = parsed.Path + case parsed.Opaque != "": + resolvedPath = parsed.Opaque + default: + resolvedPath = strings.TrimPrefix(trimmed, "file:") + if idx := strings.Index(resolvedPath, "?"); idx >= 0 { + resolvedPath = resolvedPath[:idx] + } + } + } + } + + if strings.Contains(strings.ToLower(trimmed), "mode=memory") { + return nil + } + + if unescaped, unescapeErr := url.PathUnescape(resolvedPath); unescapeErr == nil { + resolvedPath = unescaped + } + + dir := filepath.Dir(resolvedPath) + if dir == "." || dir == "" { + dir = "." + } + + hint := fmt.Sprintf("check write permissions for %s", dir) + if dir == "/data" || strings.HasPrefix(resolvedPath, "/data/") { + hint = "check ownership/permissions on ./onwatch-data (try: chown -R 65532:65532 ./onwatch-data) or use a named volume" + } + + if info, statErr := os.Stat(resolvedPath); statErr == nil { + if info.IsDir() { + return fmt.Errorf("database path points to a directory (db=%s): %s", trimmed, hint) + } + file, openErr := os.OpenFile(resolvedPath, os.O_WRONLY|os.O_APPEND, 0) + if openErr != nil { + return fmt.Errorf("database file is not writable (db=%s): %w - %s", trimmed, openErr, hint) + } + if closeErr := file.Close(); closeErr != nil { + return fmt.Errorf("database file preflight close failed (db=%s): %w", trimmed, closeErr) + } + } else if !errors.Is(statErr, os.ErrNotExist) { + return fmt.Errorf("database path preflight failed (db=%s): %w", trimmed, statErr) + } + + probe, err := os.CreateTemp(dir, ".onwatch-db-writecheck-*") + if err != nil { + return fmt.Errorf("database path is not writable (db=%s, dir=%s): %w - %s", trimmed, dir, err, hint) + } + probePath := probe.Name() + if closeErr := probe.Close(); closeErr != nil { + _ = os.Remove(probePath) + return fmt.Errorf("database path preflight failed (db=%s, dir=%s): %w", trimmed, dir, closeErr) + } + if removeErr := os.Remove(probePath); removeErr != nil { + return fmt.Errorf("database path preflight cleanup failed (db=%s, dir=%s): %w", trimmed, dir, removeErr) + } + + return nil +} + // New creates a new Store with the given database path func New(dbPath string) (*Store, error) { + if err := preflightDatabasePath(dbPath); err != nil { + return nil, err + } + db, err := sql.Open("sqlite", dbPath) if err != nil { return nil, fmt.Errorf("failed to open database: %w", err) diff --git a/internal/store/store_test.go b/internal/store/store_test.go index aba1b68..b0664a3 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -2,6 +2,11 @@ package store import ( "database/sql" + "fmt" + "os" + "path/filepath" + "runtime" + "strings" "testing" "time" @@ -105,6 +110,107 @@ func TestStore_AntigravityActiveCycleUniqueIndexExists(t *testing.T) { } } +func TestPreflightDatabasePath_MemoryPaths(t *testing.T) { + paths := []string{ + ":memory:", + "file::memory:?cache=shared", + "file:test.db?mode=memory&cache=shared", + } + + for _, path := range paths { + path := path + t.Run(path, func(t *testing.T) { + if err := preflightDatabasePath(path); err != nil { + t.Fatalf("preflightDatabasePath(%q) error = %v, want nil", path, err) + } + }) + } +} + +func TestPreflightDatabasePath_EmptyPath(t *testing.T) { + err := preflightDatabasePath(" ") + if err == nil { + t.Fatal("preflightDatabasePath should fail for empty path") + } + if !strings.Contains(err.Error(), "empty database path") { + t.Fatalf("error = %q, want empty database path", err.Error()) + } +} + +func TestPreflightDatabasePath_SQLiteFileURI(t *testing.T) { + tmpDir := t.TempDir() + dbURI := fmt.Sprintf("file:%s?cache=shared", filepath.Join(tmpDir, "uri.db")) + + if err := preflightDatabasePath(dbURI); err != nil { + t.Fatalf("preflightDatabasePath(%q) error = %v, want nil", dbURI, err) + } +} + +func TestStoreNew_SQLiteFileURI(t *testing.T) { + tmpDir := t.TempDir() + dbPath := filepath.Join(tmpDir, "uri-open.db") + dbURI := fmt.Sprintf("file:%s?cache=shared", dbPath) + + s, err := New(dbURI) + if err != nil { + t.Fatalf("New(%q) error = %v, want nil", dbURI, err) + } + defer s.Close() + + if _, err := os.Stat(dbPath); err != nil { + t.Fatalf("expected sqlite file at %s: %v", dbPath, err) + } +} + +func TestPreflightDatabasePath_UnwritableExistingFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("chmod-based permission test is not reliable on Windows") + } + + base := t.TempDir() + dbPath := filepath.Join(base, "onwatch.db") + if err := os.WriteFile(dbPath, []byte("seed"), 0o444); err != nil { + t.Fatalf("write dbPath: %v", err) + } + t.Cleanup(func() { + _ = os.Chmod(dbPath, 0o644) + }) + + err := preflightDatabasePath(dbPath) + if err == nil { + t.Fatal("preflightDatabasePath should fail for unwritable existing DB file") + } + if !strings.Contains(err.Error(), "database file is not writable") { + t.Fatalf("error = %q, want database file is not writable", err.Error()) + } +} + +func TestPreflightDatabasePath_UnwritableDirectory(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("chmod-based permission test is not reliable on Windows") + } + + base := t.TempDir() + readOnlyDir := filepath.Join(base, "readonly") + if err := os.MkdirAll(readOnlyDir, 0o755); err != nil { + t.Fatalf("mkdir readOnlyDir: %v", err) + } + if err := os.Chmod(readOnlyDir, 0o555); err != nil { + t.Fatalf("chmod readOnlyDir: %v", err) + } + t.Cleanup(func() { + _ = os.Chmod(readOnlyDir, 0o755) + }) + + err := preflightDatabasePath(filepath.Join(readOnlyDir, "onwatch.db")) + if err == nil { + t.Fatal("preflightDatabasePath should fail for unwritable directory") + } + if !strings.Contains(err.Error(), "database path is not writable") { + t.Fatalf("error = %q, want database path is not writable", err.Error()) + } +} + func TestStore_InsertSnapshot(t *testing.T) { s, err := New(":memory:") if err != nil {