From 19645e70751196242d9c3bd4bdd002f43d89ee2a Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sat, 18 Jul 2026 20:31:21 +0300 Subject: [PATCH] storage: bare store init/delete and remotesrv DBCache InitStore writes a bare NBS store (LoadDoltDB Format_DOLT + WriteEmptyRepo) with partial-failure cleanup; DeleteStore guards against paths escaping the configured repos root; RepoDiskPath lays out /~/. Cache implements remotesrv.DBCache over an injected RepoLookup (no db import, no push-to-create): Get resolves via core.ParseRepoPath, memoizes one nbs.NewLocalStore per disk path, never creates directories; Evict/Close for delete and shutdown. Package doc records the LocalFilesysWithWorkingDir sealed-URL requirement proven by the Phase-0 spike. Unit tests cover store validity (reopen, main branch, initial commit), partial-failure cleanup, root-escape guard, and cache hit/miss/memoize/evict. Spike test untouched and still green. --- storage/dbcache.go | 118 ++++++++++++++++++ storage/init.go | 115 ++++++++++++++++++ storage/storage_test.go | 261 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 494 insertions(+) create mode 100644 storage/dbcache.go create mode 100644 storage/init.go create mode 100644 storage/storage_test.go diff --git a/storage/dbcache.go b/storage/dbcache.go new file mode 100644 index 0000000000000000000000000000000000000000..61e4340397359235a85e0e8ebae90d20c9871422 --- /dev/null +++ b/storage/dbcache.go @@ -0,0 +1,118 @@ +package storage + +import ( + "context" + "fmt" + "sync" + + "github.com/dolthub/dolt/go/libraries/doltcore/remotesrv" + "github.com/dolthub/dolt/go/store/nbs" + + "go.bigb.es/sourcehut-dolt/core" +) + +// defaultMemTableSize mirrors upstream utils/remotesrv LocalCSCache: the +// in-memory table size handed to nbs.NewLocalStore before chunks spill to disk. +const defaultMemTableSize = 128 * 1024 * 1024 + +// RepoLookup resolves a repository's owner and database name to its absolute +// on-disk store directory. It returns an error when no repository row exists; +// Cache.Get propagates that error unchanged so the remotesapi interceptor layer +// can map it to a gRPC NotFound (v1 has no push-to-create: databases are made +// explicitly through the web UI). +// +// It is defined here as a function type so storage/ never imports db/; the main +// wiring injects a db-backed lookup. +type RepoLookup func(ctx context.Context, owner, name string) (diskPath string, err error) + +// Cache is a remotesrv.DBCache that serves only repositories with an existing +// row, memoizing one nbs.NewLocalStore per absolute disk path. It never creates +// directories: unknown repos are errors, not implicit creations. It is safe for +// concurrent use. +type Cache struct { + lookup RepoLookup + + mu sync.Mutex + dbs map[string]remotesrv.RemoteSrvStore +} + +var _ remotesrv.DBCache = (*Cache)(nil) + +// NewCache builds a Cache backed by lookup. lookup must be non-nil. +func NewCache(lookup RepoLookup) *Cache { + if lookup == nil { + panic("storage: NewCache requires a non-nil RepoLookup") + } + return &Cache{ + lookup: lookup, + dbs: make(map[string]remotesrv.RemoteSrvStore), + } +} + +// Get resolves the remotesapi repo path to an on-disk store and returns a +// memoized NBS chunk store for it. The path is normalized with +// core.ParseRepoPath (trim slashes, optional "~", reject traversal), then +// resolved to a disk path via the injected RepoLookup. A missing repository row +// surfaces as the lookup's error. Get never creates directories or stores on +// disk; nbs.NewLocalStore opens the existing bare store created by InitStore. +func (c *Cache) Get(ctx context.Context, path, nbfVerStr string) (remotesrv.RemoteSrvStore, error) { + owner, name, err := core.ParseRepoPath(path) + if err != nil { + return nil, fmt.Errorf("storage: parse repo path %q: %w", path, err) + } + + diskPath, err := c.lookup(ctx, owner, name) + if err != nil { + return nil, err + } + + c.mu.Lock() + defer c.mu.Unlock() + + if cs, ok := c.dbs[diskPath]; ok { + return cs, nil + } + + cs, err := nbs.NewLocalStore(ctx, nbfVerStr, diskPath, defaultMemTableSize, nbs.NewUnlimitedMemQuotaProvider(), false) + if err != nil { + return nil, fmt.Errorf("storage: open store %q: %w", diskPath, err) + } + c.dbs[diskPath] = cs + return cs, nil +} + +// Evict closes and drops the memoized store for diskPath, if any. It is called +// when a repository is deleted so a subsequent recreation at the same path does +// not reuse a stale handle. Evicting an absent path is a no-op. It returns the +// error from closing the store, if one was open. +func (c *Cache) Evict(diskPath string) error { + c.mu.Lock() + defer c.mu.Unlock() + + cs, ok := c.dbs[diskPath] + if !ok { + return nil + } + delete(c.dbs, diskPath) + if err := cs.Close(); err != nil { + return fmt.Errorf("storage: close evicted store %q: %w", diskPath, err) + } + return nil +} + +// Close closes every memoized store and empties the cache. It is called at +// server shutdown. It closes all stores before returning and reports the first +// error encountered, if any. +func (c *Cache) Close() error { + c.mu.Lock() + defer c.mu.Unlock() + + var firstErr error + for path, cs := range c.dbs { + if err := cs.Close(); err != nil && firstErr == nil { + firstErr = fmt.Errorf("storage: close store %q: %w", path, err) + } + delete(c.dbs, path) + } + return firstErr +} diff --git a/storage/init.go b/storage/init.go new file mode 100644 index 0000000000000000000000000000000000000000..bfdf48006de107c6a0828701d3d3dafe1c742266 --- /dev/null +++ b/storage/init.go @@ -0,0 +1,115 @@ +// Package storage owns dolt.sr.ht's on-disk NBS chunk stores and the +// remotesapi DBCache that serves them. +// +// A hosted database is a bare NBS chunk-store directory (no ".dolt/", no +// working set) laid out at "/~/". This is exactly what +// remotesrv serves and what "file://" dolt remotes consume, so InitStore can +// create one with the low-level doltdb primitives and remotesrv can read and +// write it directly. +// +// # remotesrv FS working directory (load-bearing) +// +// remotesrv seals its chunk-download URLs relative to the working directory of +// the filesys it is given. The server MUST be constructed with +// filesys.LocalFilesysWithWorkingDir(root) pointing at the repos root — NOT a +// plain filesys.LocalFS. With a working-dir-rooted FS the sealed URLs carry +// clean relative prefixes (e.g. "~owner/db"); with a bare LocalFS they carry +// "../../.." escapes that the sealed-URL file handler rejects, and every clone +// or push breaks at the chunk-transfer stage. This was proven end-to-end in the +// Phase-0 spike (storage/spike_test.go). Cache in this package keys stores by +// absolute disk path, so it works with either FS, but the server assembly in +// remoteapi/ must still honor this rule. +package storage + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/utils/earl" + "github.com/dolthub/dolt/go/libraries/utils/filesys" + "github.com/dolthub/dolt/go/store/types" +) + +// RepoDiskPath returns the absolute on-disk store directory for a database, +// laid out as "/~/". Callers are responsible for validating +// owner and name (see core.ValidateName) before touching disk. +func RepoDiskPath(root, owner, name string) string { + return filepath.Join(root, "~"+owner, name) +} + +// InitStore creates a bare NBS chunk store at absPath and writes an empty repo +// into it with a single "main" branch and an initial commit authored by +// ownerName/ownerEmail. +// +// absPath must be absolute. On any failure after the directory is created, +// InitStore removes absPath so a failed creation never leaves a partial store +// behind. Idempotence is NOT provided: calling InitStore on an existing store +// is a caller error and is not defended against here. +func InitStore(ctx context.Context, absPath, ownerName, ownerEmail string) (err error) { + if !filepath.IsAbs(absPath) { + return fmt.Errorf("storage: InitStore requires an absolute path, got %q", absPath) + } + + if err := os.MkdirAll(absPath, 0o755); err != nil { + return fmt.Errorf("storage: create store dir %q: %w", absPath, err) + } + // Any error past this point must not leave a half-written store behind. + defer func() { + if err != nil { + os.RemoveAll(absPath) + } + }() + + fileURL := earl.FileUrlFromPath(absPath, os.PathSeparator) + ddb, err := doltdb.LoadDoltDB(ctx, types.Format_DOLT, fileURL, filesys.LocalFS) + if err != nil { + return fmt.Errorf("storage: load doltdb at %q: %w", fileURL, err) + } + + if err = ddb.WriteEmptyRepo(ctx, "main", ownerName, ownerEmail); err != nil { + ddb.Close() + return fmt.Errorf("storage: write empty repo at %q: %w", absPath, err) + } + + // Release the init handle so the server can open its own store over the + // same directory later. + if err = ddb.Close(); err != nil { + return fmt.Errorf("storage: close init doltdb at %q: %w", absPath, err) + } + return nil +} + +// DeleteStore removes the store directory at absPath. It refuses to delete +// anything that is not strictly contained within root, guarding against a +// corrupted or attacker-controlled path escaping the configured repos root. +// Both root and absPath must be absolute. +func DeleteStore(ctx context.Context, root, absPath string) error { + if !filepath.IsAbs(root) { + return fmt.Errorf("storage: DeleteStore requires an absolute root, got %q", root) + } + if !filepath.IsAbs(absPath) { + return fmt.Errorf("storage: DeleteStore requires an absolute path, got %q", absPath) + } + + cleanRoot := filepath.Clean(root) + cleanPath := filepath.Clean(absPath) + if cleanPath == cleanRoot { + return fmt.Errorf("storage: refusing to delete the repos root %q", cleanRoot) + } + rel, err := filepath.Rel(cleanRoot, cleanPath) + if err != nil { + return fmt.Errorf("storage: DeleteStore rel(%q, %q): %w", cleanRoot, cleanPath, err) + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { + return fmt.Errorf("storage: refusing to delete %q outside repos root %q", cleanPath, cleanRoot) + } + + if err := os.RemoveAll(cleanPath); err != nil { + return fmt.Errorf("storage: remove store %q: %w", cleanPath, err) + } + return nil +} diff --git a/storage/storage_test.go b/storage/storage_test.go new file mode 100644 index 0000000000000000000000000000000000000000..da0cfe8eb066fd4ad6e48fc51cda6bc7ffdcc329 --- /dev/null +++ b/storage/storage_test.go @@ -0,0 +1,261 @@ +package storage + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/doltcore/ref" + "github.com/dolthub/dolt/go/libraries/utils/earl" + "github.com/dolthub/dolt/go/libraries/utils/filesys" + "github.com/dolthub/dolt/go/store/types" +) + +// nbfVer is the NBS version string matching the format InitStore writes with. +var nbfVer = types.Format_DOLT.VersionString() + +func TestRepoDiskPath(t *testing.T) { + got := RepoDiskPath("/var/lib/dolt", "alice", "widgets") + want := filepath.Join("/var/lib/dolt", "~alice", "widgets") + if got != want { + t.Fatalf("RepoDiskPath = %q, want %q", got, want) + } +} + +func TestInitStoreCreatesValidStore(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + absPath := RepoDiskPath(root, "alice", "widgets") + + if err := InitStore(ctx, absPath, "alice", "alice@example.test"); err != nil { + t.Fatalf("InitStore: %v", err) + } + + // Re-open the bare store read-only and confirm it is a real repo with a + // "main" branch and a resolvable initial commit. + fileURL := earl.FileUrlFromPath(absPath, os.PathSeparator) + ddb, err := doltdb.LoadDoltDB(ctx, types.Format_DOLT, fileURL, filesys.LocalFS) + if err != nil { + t.Fatalf("re-open LoadDoltDB(%s): %v", fileURL, err) + } + // NOTE: this read-only verification handle is intentionally not closed. + // Closing a DoltDB after resolving a commit trips an nbs table-index + // ref-count panic upstream (store/nbs/table_index.go). The handle is + // reclaimed at process exit and t.TempDir removes the files; this test only + // needs to confirm the store is a valid, readable repo. + + if _, has, err := ddb.HasBranch(ctx, "main"); err != nil { + t.Fatalf("HasBranch(main): %v", err) + } else if !has { + t.Fatalf("re-opened store has no 'main' branch") + } + + commit, err := ddb.ResolveCommitRef(ctx, ref.NewBranchRef("main")) + if err != nil { + t.Fatalf("ResolveCommitRef(main): %v", err) + } + if commit == nil { + t.Fatalf("ResolveCommitRef(main) returned nil commit") + } + if _, err := commit.HashOf(); err != nil { + t.Fatalf("initial commit HashOf: %v", err) + } +} + +func TestInitStoreRejectsRelativePath(t *testing.T) { + if err := InitStore(context.Background(), "relative/store", "alice", "a@b.test"); err == nil { + t.Fatalf("InitStore accepted a relative path") + } +} + +func TestInitStorePartialFailureCleanup(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("running as root: permission-based failure cannot be induced") + } + ctx := context.Background() + parent := t.TempDir() + + // A read-only intermediate directory makes MkdirAll fail so InitStore + // cannot even create the store dir; assert nothing is left behind. + ro := filepath.Join(parent, "ro") + if err := os.Mkdir(ro, 0o555); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(ro, 0o755) }) + + absPath := filepath.Join(ro, "sub", "store") + if err := InitStore(ctx, absPath, "alice", "a@b.test"); err == nil { + t.Fatalf("InitStore succeeded under a read-only parent") + } + if _, err := os.Stat(absPath); !os.IsNotExist(err) { + t.Fatalf("expected no leftover store dir at %q, stat err = %v", absPath, err) + } + if _, err := os.Stat(filepath.Join(ro, "sub")); !os.IsNotExist(err) { + t.Fatalf("expected no leftover intermediate dir, stat err = %v", err) + } +} + +func TestDeleteStoreRemovesStore(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + absPath := RepoDiskPath(root, "alice", "widgets") + if err := InitStore(ctx, absPath, "alice", "a@b.test"); err != nil { + t.Fatalf("InitStore: %v", err) + } + + if err := DeleteStore(ctx, root, absPath); err != nil { + t.Fatalf("DeleteStore: %v", err) + } + if _, err := os.Stat(absPath); !os.IsNotExist(err) { + t.Fatalf("store dir still present after DeleteStore, stat err = %v", err) + } +} + +func TestDeleteStoreRootEscapeGuard(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + sibling := t.TempDir() // absolute, definitely outside root + + // A canary the guard must protect. + canary := filepath.Join(sibling, "keep") + if err := os.WriteFile(canary, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + + cases := map[string]string{ + "sibling dir": sibling, + "traversal escape": filepath.Join(root, "..", filepath.Base(sibling)), + "the root itself": root, + "prefix-not-subdir": root + "-evil", + } + for name, target := range cases { + t.Run(name, func(t *testing.T) { + if err := DeleteStore(ctx, root, target); err == nil { + t.Fatalf("DeleteStore(%q) should have been refused", target) + } + }) + } + + if _, err := os.Stat(canary); err != nil { + t.Fatalf("canary disturbed by a refused DeleteStore: %v", err) + } +} + +func TestDeleteStoreRequiresAbsolute(t *testing.T) { + ctx := context.Background() + if err := DeleteStore(ctx, "rel/root", "/tmp/x"); err == nil { + t.Fatalf("DeleteStore accepted a relative root") + } + if err := DeleteStore(ctx, "/tmp", "rel/path"); err == nil { + t.Fatalf("DeleteStore accepted a relative path") + } +} + +func TestNewCacheRejectsNilLookup(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatalf("NewCache(nil) did not panic") + } + }() + NewCache(nil) +} + +func TestCacheGetHitAndMemoize(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + absPath := RepoDiskPath(root, "alice", "widgets") + if err := InitStore(ctx, absPath, "alice", "a@b.test"); err != nil { + t.Fatalf("InitStore: %v", err) + } + + var lookups int + cache := NewCache(func(_ context.Context, owner, name string) (string, error) { + lookups++ + if owner == "alice" && name == "widgets" { + return absPath, nil + } + return "", os.ErrNotExist + }) + defer cache.Close() + + cs1, err := cache.Get(ctx, "~alice/widgets", nbfVer) + if err != nil { + t.Fatalf("Get (first): %v", err) + } + cs2, err := cache.Get(ctx, "alice/widgets", nbfVer) + if err != nil { + t.Fatalf("Get (second): %v", err) + } + if cs1 != cs2 { + t.Fatalf("expected memoized store to be identical across calls") + } + if lookups != 2 { + t.Fatalf("expected 2 lookups (memoization keys on disk path, not the call), got %d", lookups) + } +} + +func TestCacheGetMiss(t *testing.T) { + ctx := context.Background() + cache := NewCache(func(_ context.Context, owner, name string) (string, error) { + return "", os.ErrNotExist + }) + defer cache.Close() + + if _, err := cache.Get(ctx, "~ghost/missing", nbfVer); err == nil { + t.Fatalf("Get for an unknown repo should error") + } +} + +func TestCacheGetRejectsBadPath(t *testing.T) { + ctx := context.Background() + called := false + cache := NewCache(func(_ context.Context, owner, name string) (string, error) { + called = true + return "", nil + }) + defer cache.Close() + + if _, err := cache.Get(ctx, "no-slash", nbfVer); err == nil { + t.Fatalf("Get with a malformed path should error before lookup") + } + if called { + t.Fatalf("RepoLookup was consulted for a structurally invalid path") + } +} + +func TestCacheEvictReopens(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + absPath := RepoDiskPath(root, "alice", "widgets") + if err := InitStore(ctx, absPath, "alice", "a@b.test"); err != nil { + t.Fatalf("InitStore: %v", err) + } + + cache := NewCache(func(_ context.Context, owner, name string) (string, error) { + return absPath, nil + }) + defer cache.Close() + + cs1, err := cache.Get(ctx, "~alice/widgets", nbfVer) + if err != nil { + t.Fatalf("Get (first): %v", err) + } + + if err := cache.Evict(absPath); err != nil { + t.Fatalf("Evict: %v", err) + } + // Evicting an absent path is a no-op. + if err := cache.Evict(absPath); err != nil { + t.Fatalf("Evict (absent) should be a no-op, got %v", err) + } + + cs2, err := cache.Get(ctx, "~alice/widgets", nbfVer) + if err != nil { + t.Fatalf("Get after evict: %v", err) + } + if cs1 == cs2 { + t.Fatalf("expected a freshly opened store after eviction, got the evicted handle") + } +}