diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index e76cd5b9b..3ccbb93ab 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -524,3 +524,7 @@ identifiers and metric names passed back into `Query`, `QueryAll`, and case-polluted callers cannot manufacture false "missing metrics" results, split one governed metric stream into mixed-case query buckets, or trigger redundant batch work against otherwise valid stored samples. +That same metrics-store boundary also owns the persistent DB file path. The +runtime must normalize the owned metrics directory and resolve the selected DB +filename through the shared storage-path helper before it creates directories +or opens SQLite, instead of trusting raw caller-built paths. diff --git a/pkg/metrics/store.go b/pkg/metrics/store.go index ead9284d8..2df0727e4 100644 --- a/pkg/metrics/store.go +++ b/pkg/metrics/store.go @@ -17,6 +17,7 @@ import ( "github.com/rs/zerolog/log" _ "modernc.org/sqlite" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" pdb "github.com/rcourtman/pulse-go-rewrite/pkg/db" ) @@ -51,8 +52,13 @@ type StoreConfig struct { // DefaultConfig returns sensible defaults for metrics storage func DefaultConfig(dataDir string) StoreConfig { + dbPath := filepath.Join(dataDir, "metrics.db") + if resolvedDBPath, err := resolveStoreDBPath(dbPath); err == nil { + dbPath = resolvedDBPath + } + return StoreConfig{ - DBPath: filepath.Join(dataDir, "metrics.db"), + DBPath: dbPath, WriteBufferSize: 100, FlushInterval: 5 * time.Second, RetentionRaw: 2 * time.Hour, @@ -62,6 +68,26 @@ func DefaultConfig(dataDir string) StoreConfig { } } +func resolveStoreDBPath(dbPath string) (string, error) { + trimmedPath := strings.TrimSpace(dbPath) + if trimmedPath == "" { + return "", fmt.Errorf("metrics database path is required") + } + + cleanedPath := filepath.Clean(trimmedPath) + dir, err := securityutil.NormalizeStorageDir(filepath.Dir(cleanedPath)) + if err != nil { + return "", fmt.Errorf("resolve metrics database directory: %w", err) + } + + resolvedPath, err := securityutil.JoinStorageLeaf(dir, filepath.Base(cleanedPath)) + if err != nil { + return "", fmt.Errorf("resolve metrics database path: %w", err) + } + + return resolvedPath, nil +} + // bufferedMetric holds a metric waiting to be written type bufferedMetric struct { resourceType string @@ -101,6 +127,12 @@ type Store struct { // NewStore creates a new metrics store with the given configuration func NewStore(config StoreConfig) (*Store, error) { + resolvedDBPath, err := resolveStoreDBPath(config.DBPath) + if err != nil { + return nil, err + } + config.DBPath = resolvedDBPath + // Ensure directory exists dir := filepath.Dir(config.DBPath) if err := os.MkdirAll(dir, 0755); err != nil { diff --git a/pkg/metrics/store_additional_test.go b/pkg/metrics/store_additional_test.go index e8918c084..4ef272b4f 100644 --- a/pkg/metrics/store_additional_test.go +++ b/pkg/metrics/store_additional_test.go @@ -2,6 +2,7 @@ package metrics import ( "bytes" + "os" "path/filepath" "strings" "testing" @@ -37,6 +38,47 @@ func TestStoreWriteBatchSync(t *testing.T) { } } +func TestResolveStoreDBPathCanonicalizesOwnedPath(t *testing.T) { + root := t.TempDir() + rawPath := filepath.Join(root, "metrics", "..", "metrics", "metrics.db") + + resolved, err := resolveStoreDBPath(" " + rawPath + " ") + if err != nil { + t.Fatalf("resolveStoreDBPath() error = %v", err) + } + + want := filepath.Join(filepath.Clean(filepath.Join(root, "metrics")), "metrics.db") + if resolved != want { + t.Fatalf("resolveStoreDBPath() = %q, want %q", resolved, want) + } +} + +func TestResolveStoreDBPathRejectsBlank(t *testing.T) { + if _, err := resolveStoreDBPath(" \t "); err == nil { + t.Fatal("expected blank DB path to be rejected") + } +} + +func TestNewStoreCanonicalizesDBPath(t *testing.T) { + root := t.TempDir() + cfg := DefaultConfig(root) + cfg.DBPath = filepath.Join(root, "metrics", "..", "metrics", "tenant-metrics.db") + + store, err := NewStore(cfg) + if err != nil { + t.Fatalf("NewStore() error = %v", err) + } + defer store.Close() + + want := filepath.Join(filepath.Clean(filepath.Join(root, "metrics")), "tenant-metrics.db") + if store.config.DBPath != want { + t.Fatalf("store.config.DBPath = %q, want %q", store.config.DBPath, want) + } + if _, err := os.Stat(want); err != nil { + t.Fatalf("expected canonical metrics DB at %q: %v", want, err) + } +} + func TestStoreClear(t *testing.T) { dir := t.TempDir() store, err := NewStore(DefaultConfig(dir))