From ba34443357cb2471337ae8faa7dee369bdbcae69 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sat, 18 Jul 2026 23:26:37 +0300 Subject: [PATCH] feat(remoteapi): auto-create databases on first push to own namespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Push-to-create: an authenticated, non-suspended caller pushing (or cloning) an unknown repo under their OWN namespace has it transparently created — a PRIVATE repository row plus a genuinely empty on-disk NBS store — then proceeds through the normal ACL check as the owner. Any other case (anonymous, suspended, another user's namespace, invalid name) still returns NotFound, so a stranger's namespace is never leaked and nothing is created. storage.InitEmptyStore creates the store WITHOUT WriteEmptyRepo: an "Initialize data repository" commit would make the client's first push a non-fast-forward and be rejected. An empty store (root = empty hash) lets the initial push land as the repo's first history. The interceptor auto-create is race-safe (ErrNameTaken re-fetch) and rolls the row back if the store cannot be created. Proven end-to-end (integration): a real `dolt push` to a new name auto-creates PRIVATE + fast-forwards + re-clones; a foreign-namespace push is denied with no row created. All prior clone/push/ACL scenarios still pass. --- remoteapi/integration_test.go | 69 ++++++++++ remoteapi/interceptors.go | 94 ++++++++++++-- remoteapi/interceptors_test.go | 223 +++++++++++++++++++++++++++++---- remoteapi/server.go | 10 +- storage/init.go | 39 ++++++ storage/initempty_test.go | 48 +++++++ 6 files changed, 446 insertions(+), 37 deletions(-) create mode 100644 storage/initempty_test.go diff --git a/remoteapi/integration_test.go b/remoteapi/integration_test.go index 22eba90334673926a2b2dccbcc905e0a3be5b933..c109c4732369f1a6cb083b5f33d470c0da06df5a 100644 --- a/remoteapi/integration_test.go +++ b/remoteapi/integration_test.go @@ -321,6 +321,75 @@ func TestRemoteAPIIntegration(t *testing.T) { } t.Logf("RO grantee push correctly denied:\n%s", out) }) + + // (8) Push-to-create: alice pushes to a brand-new db that has no repository + // row and no store; the server auto-creates a PRIVATE repo + an empty store + // and the initial push (a fast-forward from the empty root) succeeds. This is + // the end-to-end proof that InitEmptyStore does NOT write an initial commit — + // an "Initialize data repository" commit on the remote would make this a + // non-fast-forward and the push would be rejected. + t.Run("push_to_create", func(t *testing.T) { + newURL := fmt.Sprintf("http://%s/~alice/created", addr) + env := append(append([]string{}, baseEnv...), "DOLT_REMOTE_PASSWORD="+pat) + + // A fresh local db (dolt init writes its own initial commit) to push from. + work := filepath.Join(t.TempDir(), "created") + if err := os.MkdirAll(work, 0o755); err != nil { + t.Fatal(err) + } + runDolt(t, env, work, "init") + runDolt(t, env, work, "sql", "-q", "create table made(i int primary key); insert into made values (42);") + runDolt(t, env, work, "commit", "-Am", "created via push") + runDolt(t, env, work, "remote", "add", "origin", newURL) + runDolt(t, env, work, "push", "--user", "alice", "origin", "main") + + // The repository row now exists: PRIVATE, owned by alice. + created, err := store.GetRepoByOwnerAndName(ctx, "alice", "created") + if err != nil { + t.Fatalf("auto-created repo row missing after push: %v", err) + } + if created.Visibility != core.VisibilityPrivate { + t.Fatalf("auto-created repo visibility = %s, want PRIVATE", created.Visibility) + } + if created.OwnerID != 1 { + t.Fatalf("auto-created repo owner = %d, want 1 (alice)", created.OwnerID) + } + + // It is PRIVATE, so re-clone with alice's PAT and verify the row landed. + dir2 := t.TempDir() + runDolt(t, env, dir2, "clone", "--user", "alice", newURL) + out := runDolt(t, env, filepath.Join(dir2, "created"), "sql", "-q", "select i from made", "-r", "csv") + if !strings.Contains(out, "42") { + t.Fatalf("pushed row not present after re-clone; got:\n%s", out) + } + t.Log("push-to-create succeeded and re-clone returned the pushed row") + }) + + // (9) Push-to-create is refused in another user's namespace: bob (valid RW + // PAT) cannot create under ~alice. The push fails and no row is created — the + // owner-only guard, which also avoids leaking a stranger's namespace. + t.Run("push_to_create_foreign_namespace_denied", func(t *testing.T) { + foreignURL := fmt.Sprintf("http://%s/~alice/bobtried", addr) + bobPAT := forgePAT("bob", "dolt.sr.ht/repos:RW", time.Now().Add(time.Hour)) + env := append(append([]string{}, baseEnv...), "DOLT_REMOTE_PASSWORD="+bobPAT) + + work := filepath.Join(t.TempDir(), "bobtried") + if err := os.MkdirAll(work, 0o755); err != nil { + t.Fatal(err) + } + runDolt(t, env, work, "init") + runDolt(t, env, work, "sql", "-q", "create table t(i int primary key); insert into t values (1);") + runDolt(t, env, work, "commit", "-Am", "bob change") + runDolt(t, env, work, "remote", "add", "origin", foreignURL) + out, err := tryDolt(env, work, "push", "--user", "bob", "origin", "main") + if err == nil { + t.Fatalf("bob push-to-create under ~alice unexpectedly succeeded:\n%s", out) + } + if _, err := store.GetRepoByOwnerAndName(ctx, "alice", "bobtried"); err == nil { + t.Fatalf("foreign-namespace push must not create a repository row") + } + t.Logf("foreign-namespace push-to-create correctly denied:\n%s", out) + }) } // --- helpers --- diff --git a/remoteapi/interceptors.go b/remoteapi/interceptors.go index d3405ed102d657bf0d9a51929fdca2e73dbf8ef1..60d2342619dc8963b888f8eeb15d2de88c98875d 100644 --- a/remoteapi/interceptors.go +++ b/remoteapi/interceptors.go @@ -6,9 +6,6 @@ import ( "errors" "fmt" - "sourcecraft.dev/bigbes/sr-ht-core/auth" - "sourcecraft.dev/bigbes/sr-ht-core/config" - "sourcecraft.dev/bigbes/sr-ht-core/database" remotesapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/remotesapi/v1alpha1" "github.com/sirupsen/logrus" "github.com/vaughan0/go-ini" @@ -16,10 +13,14 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" + "sourcecraft.dev/bigbes/sr-ht-core/auth" + "sourcecraft.dev/bigbes/sr-ht-core/config" + "sourcecraft.dev/bigbes/sr-ht-core/database" "sourcecraft.dev/bigbes/sr-ht-dolt/authn" "sourcecraft.dev/bigbes/sr-ht-dolt/core" "sourcecraft.dev/bigbes/sr-ht-dolt/db" + "sourcecraft.dev/bigbes/sr-ht-dolt/storage" ) // Method-classification sets, copied verbatim from upstream remotesrv's @@ -71,6 +72,12 @@ type repoRequest interface { type repoStore interface { GetRepoByOwnerAndName(ctx context.Context, ownerUsername, name string) (*core.Repo, error) EffectiveAccess(ctx context.Context, userID, repoID int) (*core.AccessMode, error) + // CreateRepo and DeleteRepo back push-to-create: an authenticated caller + // touching a not-yet-existing repo in their own namespace has the row + // inserted (CreateRepo) and, if the on-disk store then fails to materialize, + // rolled back (DeleteRepo). + CreateRepo(ctx context.Context, r *core.Repo) (*core.Repo, error) + DeleteRepo(ctx context.Context, id int) error } // interceptor holds the collaborators the per-RPC auth/authz decision needs. It @@ -90,18 +97,29 @@ type interceptor struct { // stores returns a repoStore for a request. Production binds a fresh // db.Store to the shared pool; tests inject a stub. stores func() repoStore + + // reposRoot is the absolute directory under which bare NBS stores live; it + // resolves the on-disk path for a push-to-created repository. + reposRoot string + + // createStore materializes the empty on-disk store for a push-to-created + // repository. Production uses storage.InitEmptyStore; tests inject a fake. + createStore func(ctx context.Context, absPath string) error } // newInterceptor builds the interceptor over the shared pool. It binds a fresh // db.Store per request (the pool owns connection lifetime, ctx bounds each // query). pool and keys must be non-nil. -func newInterceptor(conf ini.File, service, expectedAud string, pool *sql.DB, keys authn.KeyStore, logger *logrus.Entry) *interceptor { +func newInterceptor(conf ini.File, service, expectedAud string, pool *sql.DB, keys authn.KeyStore, reposRoot string, createStore func(ctx context.Context, absPath string) error, logger *logrus.Entry) *interceptor { if pool == nil { panic("remoteapi: newInterceptor requires a non-nil *sql.DB") } if logger == nil { logger = logrus.NewEntry(logrus.StandardLogger()) } + if createStore == nil { + createStore = storage.InitEmptyStore + } i := &interceptor{ conf: conf, service: service, @@ -109,6 +127,8 @@ func newInterceptor(conf ini.File, service, expectedAud string, pool *sql.DB, ke keys: keys, logger: logger, pool: pool, + reposRoot: reposRoot, + createStore: createStore, } i.stores = func() repoStore { return db.NewStore(pool) } return i @@ -218,17 +238,32 @@ func (i *interceptor) authorize(ctx context.Context, ac *auth.AuthContext, fullM return nil, status.Errorf(codes.InvalidArgument, "invalid repository path %q: %v", repoPath, err) } + caller := authn.AsCoreCaller(ac) + store := i.stores() repo, err := store.GetRepoByOwnerAndName(ctx, owner, name) if err != nil { if errors.Is(err, db.ErrNotFound) { - return nil, status.Errorf(codes.NotFound, "repository %s/%s not found", owner, name) + // Push-to-create: an authenticated, non-suspended caller touching a + // not-yet-existing repo in THEIR OWN namespace (with a valid name) + // has it transparently created — a PRIVATE row plus a genuinely + // empty on-disk store — then proceeds through the normal ACL check + // as the owner. Any other case (anonymous, suspended, another + // user's namespace, invalid name) keeps returning NotFound, which + // also avoids leaking existence of a stranger's private repos. + if caller == nil || caller.Suspended || owner != caller.Username || core.ValidateName(name) != nil { + return nil, status.Errorf(codes.NotFound, "repository %s/%s not found", owner, name) + } + repo, err = i.autoCreate(ctx, store, caller, owner, name) + if err != nil { + return nil, err + } + } else { + i.logger.Errorf("remotesapi repo lookup %s/%s: %v", owner, name, err) + return nil, status.Error(codes.Unavailable, "repository lookup temporarily unavailable") } - i.logger.Errorf("remotesapi repo lookup %s/%s: %v", owner, name, err) - return nil, status.Error(codes.Unavailable, "repository lookup temporarily unavailable") } - caller := authn.AsCoreCaller(ac) var aclMode *core.AccessMode if caller != nil { aclMode, err = store.EffectiveAccess(ctx, caller.UserID, repo.ID) @@ -248,6 +283,49 @@ func (i *interceptor) authorize(ctx context.Context, ac *auth.AuthContext, fullM return authn.WithCaller(ctx, ac), nil } +// autoCreate transparently creates a PRIVATE repository owned by caller under +// owner/name (push-to-create). It inserts the row and materializes a genuinely +// empty on-disk store. A concurrent first-push that already created the row is +// tolerated: the caller loses the CreateRepo race, re-fetches the winner's row, +// and does NOT re-create the store. If the store fails to materialize, the row +// is rolled back so a later push retries cleanly. Preconditions (authenticated, +// non-suspended, owns the namespace, valid name) are checked by the caller. +func (i *interceptor) autoCreate(ctx context.Context, store repoStore, caller *core.Caller, owner, name string) (*core.Repo, error) { + newRepo := &core.Repo{ + Name: name, + OwnerID: caller.UserID, + OwnerName: owner, + Path: storage.RepoDiskPath(i.reposRoot, owner, name), + Visibility: core.VisibilityPrivate, + } + created, cerr := store.CreateRepo(ctx, newRepo) + if cerr != nil { + if errors.Is(cerr, db.ErrNameTaken) { + // Lost the race: another first-push created the row (and its store). + // Adopt the winner's row; do not touch disk. + repo, gerr := store.GetRepoByOwnerAndName(ctx, owner, name) + if gerr != nil { + i.logger.Errorf("remotesapi auto-create refetch %s/%s: %v", owner, name, gerr) + return nil, status.Error(codes.Unavailable, "repository creation temporarily unavailable") + } + return repo, nil + } + i.logger.Errorf("remotesapi auto-create %s/%s: %v", owner, name, cerr) + return nil, status.Error(codes.Unavailable, "repository creation temporarily unavailable") + } + + if serr := i.createStore(ctx, created.Path); serr != nil { + // The row exists but the store does not: roll the row back (best effort) + // so the repo does not linger half-created and a retry can succeed. + if derr := store.DeleteRepo(ctx, created.ID); derr != nil { + i.logger.Errorf("remotesapi auto-create rollback %s/%s (id=%d): %v", owner, name, created.ID, derr) + } + i.logger.Errorf("remotesapi auto-create store %s/%s: %v", owner, name, serr) + return nil, status.Error(codes.Unavailable, "repository creation temporarily unavailable") + } + return created, nil +} + // repoPathOf extracts the target repo path from a request without panicking on // an absent path (upstream's getRepoPath panics on empty path + nil repo id). // It returns "" when neither is present so the caller can treat Root specially. diff --git a/remoteapi/interceptors_test.go b/remoteapi/interceptors_test.go index 5fc4d8e38457c9d854c97428da7e27e4f4d44299..653000db02dc848595b8ce6af0cbddb520125de8 100644 --- a/remoteapi/interceptors_test.go +++ b/remoteapi/interceptors_test.go @@ -2,22 +2,28 @@ package remoteapi import ( "context" + "errors" "testing" - "sourcecraft.dev/bigbes/sr-ht-core/auth" - "sourcecraft.dev/bigbes/sr-ht-core/config" remotesapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/remotesapi/v1alpha1" "github.com/sirupsen/logrus" "github.com/vaughan0/go-ini" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "sourcecraft.dev/bigbes/sr-ht-core/auth" + "sourcecraft.dev/bigbes/sr-ht-core/config" "sourcecraft.dev/bigbes/sr-ht-dolt/authn" "sourcecraft.dev/bigbes/sr-ht-dolt/core" "sourcecraft.dev/bigbes/sr-ht-dolt/db" + "sourcecraft.dev/bigbes/sr-ht-dolt/storage" ) +// errBoom is a sentinel failure injected into createStore to exercise the +// push-to-create rollback path. +var errBoom = errors.New("boom") + const ( mRoot = "/dolt.services.remotesapi.v1alpha1.ChunkStoreService/Root" mGetMeta = "/dolt.services.remotesapi.v1alpha1.ChunkStoreService/GetRepoMetadata" @@ -37,25 +43,64 @@ type fakeReq struct { func (f fakeReq) GetRepoPath() string { return f.path } func (f fakeReq) GetRepoId() *remotesapi.RepoId { return f.id } -// stubStore is an in-memory repoStore: no Postgres. +// stubStore is an in-memory repoStore: no Postgres. It is a pointer receiver so +// tests can inspect recorded auto-create calls after authorize runs. type stubStore struct { repo *core.Repo repoErr error acl *core.AccessMode aclErr error + + // createErr, when set, is returned by CreateRepo (e.g. db.ErrNameTaken to + // simulate losing the first-push race). + createErr error + // refetchRepo is returned by GetRepoByOwnerAndName after a CreateRepo that + // failed with ErrNameTaken (the concurrent winner's row). When nil, repo is + // returned instead. + refetchRepo *core.Repo + + // Recorded calls. + created *core.Repo // the repo passed to CreateRepo + createCalls int + deletedID int + deleteCalls int + getCalls int } -func (s stubStore) GetRepoByOwnerAndName(_ context.Context, _, _ string) (*core.Repo, error) { +func (s *stubStore) GetRepoByOwnerAndName(_ context.Context, _, _ string) (*core.Repo, error) { + s.getCalls++ + // After a CreateRepo lost the ErrNameTaken race, the re-fetch resolves the + // concurrent winner's row. + if s.createCalls > 0 && s.refetchRepo != nil { + return s.refetchRepo, nil + } if s.repoErr != nil { return nil, s.repoErr } return s.repo, nil } -func (s stubStore) EffectiveAccess(_ context.Context, _, _ int) (*core.AccessMode, error) { +func (s *stubStore) EffectiveAccess(_ context.Context, _, _ int) (*core.AccessMode, error) { return s.acl, s.aclErr } +func (s *stubStore) CreateRepo(_ context.Context, r *core.Repo) (*core.Repo, error) { + s.createCalls++ + s.created = r + if s.createErr != nil { + return nil, s.createErr + } + out := *r + out.ID = 999 + return &out, nil +} + +func (s *stubStore) DeleteRepo(_ context.Context, id int) error { + s.deleteCalls++ + s.deletedID = id + return nil +} + func testInterceptor(store repoStore) *interceptor { return &interceptor{ conf: ini.File{}, @@ -63,6 +108,8 @@ func testInterceptor(store repoStore) *interceptor { expectedAud: "dolt.srht.bigb.es", logger: logrus.NewEntry(logrus.New()), stores: func() repoStore { return store }, + reposRoot: "/tmp/repos", + createStore: func(context.Context, string) error { return nil }, } } @@ -137,77 +184,77 @@ func TestAuthorize(t *testing.T) { req := fakeReq{path: "~alice/db"} t.Run("anonymous public read allowed", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 100, core.VisibilityPublic)}) + i := testInterceptor(&stubStore{repo: repo(1, 100, core.VisibilityPublic)}) if _, err := i.authorize(ctx, nil, mGetMeta, req); err != nil { t.Fatalf("unexpected error: %v", err) } }) t.Run("anonymous public push denied (visible)", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 100, core.VisibilityPublic)}) + i := testInterceptor(&stubStore{repo: repo(1, 100, core.VisibilityPublic)}) _, err := i.authorize(ctx, nil, mCommit, req) wantCode(t, err, codes.PermissionDenied) }) t.Run("anonymous private read is not found", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 100, core.VisibilityPrivate)}) + i := testInterceptor(&stubStore{repo: repo(1, 100, core.VisibilityPrivate)}) _, err := i.authorize(ctx, nil, mGetMeta, req) wantCode(t, err, codes.NotFound) }) t.Run("owner push allowed", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 42, core.VisibilityPrivate)}) + i := testInterceptor(&stubStore{repo: repo(1, 42, core.VisibilityPrivate)}) if _, err := i.authorize(ctx, cookieCaller(42), mCommit, req); err != nil { t.Fatalf("unexpected error: %v", err) } }) t.Run("non-owner private read no acl is not found", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 42, core.VisibilityPrivate)}) + i := testInterceptor(&stubStore{repo: repo(1, 42, core.VisibilityPrivate)}) _, err := i.authorize(ctx, cookieCaller(7), mGetMeta, req) wantCode(t, err, codes.NotFound) }) t.Run("acl RO reads private", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 42, core.VisibilityPrivate), acl: acl(core.AccessRO)}) + i := testInterceptor(&stubStore{repo: repo(1, 42, core.VisibilityPrivate), acl: acl(core.AccessRO)}) if _, err := i.authorize(ctx, cookieCaller(7), mGetMeta, req); err != nil { t.Fatalf("unexpected error: %v", err) } }) t.Run("acl RO cannot push private (visible ⇒ PermissionDenied)", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 42, core.VisibilityPrivate), acl: acl(core.AccessRO)}) + i := testInterceptor(&stubStore{repo: repo(1, 42, core.VisibilityPrivate), acl: acl(core.AccessRO)}) _, err := i.authorize(ctx, cookieCaller(7), mCommit, req) wantCode(t, err, codes.PermissionDenied) }) t.Run("acl RW pushes private", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 42, core.VisibilityPrivate), acl: acl(core.AccessRW)}) + i := testInterceptor(&stubStore{repo: repo(1, 42, core.VisibilityPrivate), acl: acl(core.AccessRW)}) if _, err := i.authorize(ctx, cookieCaller(7), mCommit, req); err != nil { t.Fatalf("unexpected error: %v", err) } }) t.Run("unknown method denied", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 100, core.VisibilityPublic)}) + i := testInterceptor(&stubStore{repo: repo(1, 100, core.VisibilityPublic)}) _, err := i.authorize(ctx, nil, mUnknown, req) wantCode(t, err, codes.PermissionDenied) }) t.Run("malformed path is invalid argument", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 100, core.VisibilityPublic)}) + i := testInterceptor(&stubStore{repo: repo(1, 100, core.VisibilityPublic)}) _, err := i.authorize(ctx, nil, mGetMeta, fakeReq{path: "no-slash"}) wantCode(t, err, codes.InvalidArgument) }) t.Run("missing repo row is not found", func(t *testing.T) { - i := testInterceptor(stubStore{repoErr: db.ErrNotFound}) + i := testInterceptor(&stubStore{repoErr: db.ErrNotFound}) _, err := i.authorize(ctx, nil, mGetMeta, req) wantCode(t, err, codes.NotFound) }) t.Run("root ping with no path is allowed", func(t *testing.T) { - i := testInterceptor(stubStore{}) + i := testInterceptor(&stubStore{}) got, err := i.authorize(ctx, nil, mRoot, fakeReq{}) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -218,13 +265,13 @@ func TestAuthorize(t *testing.T) { }) t.Run("non-root with no path is invalid argument", func(t *testing.T) { - i := testInterceptor(stubStore{}) + i := testInterceptor(&stubStore{}) _, err := i.authorize(ctx, nil, mGetMeta, fakeReq{}) wantCode(t, err, codes.InvalidArgument) }) t.Run("repo id shape resolves path", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 100, core.VisibilityPublic)}) + i := testInterceptor(&stubStore{repo: repo(1, 100, core.VisibilityPublic)}) r := fakeReq{id: &remotesapi.RepoId{Org: "alice", RepoName: "db"}} if _, err := i.authorize(ctx, nil, mGetMeta, r); err != nil { t.Fatalf("unexpected error: %v", err) @@ -237,39 +284,165 @@ func TestAuthorizeGrantGate(t *testing.T) { req := fakeReq{path: "~alice/db"} t.Run("PAT without RO grant denied on read", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 42, core.VisibilityPublic)}) + i := testInterceptor(&stubStore{repo: repo(1, 42, core.VisibilityPublic)}) _, err := i.authorize(ctx, patCaller(t, 42, "meta.sr.ht/profile:RO"), mGetMeta, req) wantCode(t, err, codes.PermissionDenied) }) t.Run("PAT with RO grant allowed on read of own repo", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 42, core.VisibilityPublic)}) + i := testInterceptor(&stubStore{repo: repo(1, 42, core.VisibilityPublic)}) if _, err := i.authorize(ctx, patCaller(t, 42, "dolt.sr.ht/repos:RO"), mGetMeta, req); err != nil { t.Fatalf("unexpected error: %v", err) } }) t.Run("PAT with RO grant cannot push", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 42, core.VisibilityPublic)}) + i := testInterceptor(&stubStore{repo: repo(1, 42, core.VisibilityPublic)}) _, err := i.authorize(ctx, patCaller(t, 42, "dolt.sr.ht/repos:RO"), mCommit, req) wantCode(t, err, codes.PermissionDenied) }) t.Run("PAT with RW grant pushes own repo", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 42, core.VisibilityPublic)}) + i := testInterceptor(&stubStore{repo: repo(1, 42, core.VisibilityPublic)}) if _, err := i.authorize(ctx, patCaller(t, 42, "dolt.sr.ht/repos:RW"), mCommit, req); err != nil { t.Fatalf("unexpected error: %v", err) } }) t.Run("universal PAT (empty grants) passes gate", func(t *testing.T) { - i := testInterceptor(stubStore{repo: repo(1, 42, core.VisibilityPublic)}) + i := testInterceptor(&stubStore{repo: repo(1, 42, core.VisibilityPublic)}) if _, err := i.authorize(ctx, patCaller(t, 42, ""), mCommit, req); err != nil { t.Fatalf("unexpected error: %v", err) } }) } +// TestAuthorizePushToCreate covers the push-to-create branch of authorize: an +// authenticated owner touching a not-yet-existing repo in their own namespace +// transparently creates it (PRIVATE row + empty on-disk store), while every +// other case keeps returning NotFound and creates nothing. +func TestAuthorizePushToCreate(t *testing.T) { + ctx := context.Background() + req := fakeReq{path: "~alice/newdb"} + + t.Run("owner create on GetRepoMetadata read", func(t *testing.T) { + st := &stubStore{repoErr: db.ErrNotFound} + var gotPath string + i := testInterceptor(st) + i.createStore = func(_ context.Context, p string) error { gotPath = p; return nil } + + // The first RPC of a push is GetRepoMetadata (a read); auto-create fires. + if _, err := i.authorize(ctx, cookieCaller(42), mGetMeta, req); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if st.createCalls != 1 { + t.Fatalf("CreateRepo calls = %d, want 1", st.createCalls) + } + if st.created == nil || st.created.Visibility != core.VisibilityPrivate { + t.Fatalf("created repo visibility = %v, want PRIVATE", st.created) + } + if st.created.OwnerID != 42 || st.created.Name != "newdb" || st.created.OwnerName != "alice" { + t.Fatalf("created repo = %+v, want owner 42 alice/newdb", st.created) + } + wantPath := storage.RepoDiskPath("/tmp/repos", "alice", "newdb") + if st.created.Path != wantPath || gotPath != wantPath { + t.Fatalf("store path: created=%q createStore=%q, want %q", st.created.Path, gotPath, wantPath) + } + if st.deleteCalls != 0 { + t.Fatalf("DeleteRepo calls = %d, want 0", st.deleteCalls) + } + }) + + t.Run("owner create then push allowed", func(t *testing.T) { + st := &stubStore{repoErr: db.ErrNotFound} + i := testInterceptor(st) + if _, err := i.authorize(ctx, cookieCaller(42), mCommit, req); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if st.createCalls != 1 { + t.Fatalf("CreateRepo calls = %d, want 1", st.createCalls) + } + }) + + t.Run("name-taken race re-fetches, no store creation", func(t *testing.T) { + winner := &core.Repo{ID: 7, Name: "newdb", OwnerID: 42, OwnerName: "alice", Visibility: core.VisibilityPrivate} + st := &stubStore{repoErr: db.ErrNotFound, createErr: db.ErrNameTaken, refetchRepo: winner} + var storeCreated bool + i := testInterceptor(st) + i.createStore = func(context.Context, string) error { storeCreated = true; return nil } + + if _, err := i.authorize(ctx, cookieCaller(42), mCommit, req); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if st.createCalls != 1 { + t.Fatalf("CreateRepo calls = %d, want 1", st.createCalls) + } + if storeCreated { + t.Fatal("createStore must not be called when the row already exists") + } + if st.deleteCalls != 0 { + t.Fatalf("DeleteRepo calls = %d, want 0", st.deleteCalls) + } + }) + + t.Run("store creation failure rolls back row", func(t *testing.T) { + st := &stubStore{repoErr: db.ErrNotFound} + i := testInterceptor(st) + i.createStore = func(context.Context, string) error { return errBoom } + + _, err := i.authorize(ctx, cookieCaller(42), mCommit, req) + wantCode(t, err, codes.Unavailable) + if st.deleteCalls != 1 || st.deletedID != 999 { + t.Fatalf("rollback: deleteCalls=%d deletedID=%d, want 1 and 999", st.deleteCalls, st.deletedID) + } + }) + + t.Run("non-owner namespace is not found, no create", func(t *testing.T) { + st := &stubStore{repoErr: db.ErrNotFound} + i := testInterceptor(st) + // caller bob touching ~alice/newdb. + bob := &auth.AuthContext{UserID: 7, Username: "bob", UserType: auth.USER_TYPE_USER, AuthMethod: auth.AUTH_COOKIE} + _, err := i.authorize(ctx, bob, mCommit, req) + wantCode(t, err, codes.NotFound) + if st.createCalls != 0 { + t.Fatalf("CreateRepo calls = %d, want 0", st.createCalls) + } + }) + + t.Run("anonymous is not found, no create", func(t *testing.T) { + st := &stubStore{repoErr: db.ErrNotFound} + i := testInterceptor(st) + _, err := i.authorize(ctx, nil, mCommit, req) + wantCode(t, err, codes.NotFound) + if st.createCalls != 0 { + t.Fatalf("CreateRepo calls = %d, want 0", st.createCalls) + } + }) + + t.Run("suspended owner is not found, no create", func(t *testing.T) { + st := &stubStore{repoErr: db.ErrNotFound} + i := testInterceptor(st) + suspended := &auth.AuthContext{UserID: 42, Username: "alice", UserType: auth.USER_TYPE_SUSPENDED, AuthMethod: auth.AUTH_COOKIE} + _, err := i.authorize(ctx, suspended, mCommit, req) + wantCode(t, err, codes.NotFound) + if st.createCalls != 0 { + t.Fatalf("CreateRepo calls = %d, want 0", st.createCalls) + } + }) + + t.Run("invalid name is not found, no create", func(t *testing.T) { + st := &stubStore{repoErr: db.ErrNotFound} + i := testInterceptor(st) + // "bad name" passes ParseRepoPath (2 non-traversal segments) but fails + // core.ValidateName (space is not an allowed character). + _, err := i.authorize(ctx, cookieCaller(42), mCommit, fakeReq{path: "~alice/bad name"}) + wantCode(t, err, codes.NotFound) + if st.createCalls != 0 { + t.Fatalf("CreateRepo calls = %d, want 0", st.createCalls) + } + }) +} + // TestUnaryAnonymous exercises the full unary path (authenticate + authorize) // for an anonymous request: no metadata ⇒ anonymous caller, no crypto/PG. func TestUnaryAnonymous(t *testing.T) { @@ -277,7 +450,7 @@ func TestUnaryAnonymous(t *testing.T) { conf: ini.File{}, service: "dolt.sr.ht", logger: logrus.NewEntry(logrus.New()), - stores: func() repoStore { return stubStore{repo: repo(1, 100, core.VisibilityPublic)} }, + stores: func() repoStore { return &stubStore{repo: repo(1, 100, core.VisibilityPublic)} }, } var gotCaller bool handler := func(ctx context.Context, _ any) (any, error) { diff --git a/remoteapi/server.go b/remoteapi/server.go index cb8c12a370d252022801994fb85835b4dabd08ed..b78d0fb36febdc816bc7da29083a25f144ebd674 100644 --- a/remoteapi/server.go +++ b/remoteapi/server.go @@ -78,9 +78,11 @@ func New(cfg Config) (*Server, error) { } // Repo lookup: resolve owner/name to the absolute on-disk store dir via the - // repository row. v1 has no push-to-create, so a missing row is an error the - // cache propagates (mapped to gRPC NotFound at the interceptor layer, which - // runs first anyway). + // repository row. By the time Cache.Get runs, the row already exists: the + // interceptor runs first on every RPC and, for an authenticated owner + // touching a new name in their own namespace, has auto-created the row and + // its empty store (push-to-create). A still-missing row here is therefore a + // genuine not-found, propagated to the cache's caller. lookup := func(ctx context.Context, owner, name string) (string, error) { repo, err := db.NewStore(cfg.DB).GetRepoByOwnerAndName(ctx, owner, name) if err != nil { @@ -91,7 +93,7 @@ func New(cfg Config) (*Server, error) { cache := storage.NewCache(lookup) keys := newKeyStore(cfg.DB) - icept := newInterceptor(cfg.Conf, serviceName, normalizeAud(cfg.HttpHost), cfg.DB, keys, logger) + icept := newInterceptor(cfg.Conf, serviceName, normalizeAud(cfg.HttpHost), cfg.DB, keys, cfg.ReposRoot, storage.InitEmptyStore, logger) // Load-bearing (see storage/init.go): the FS MUST be rooted at ReposRoot via // LocalFilesysWithWorkingDir so sealed chunk-download URLs carry clean diff --git a/storage/init.go b/storage/init.go index bfdf48006de107c6a0828701d3d3dafe1c742266..de293260525c2fed6d0ffb7cc81d58a4124bd914 100644 --- a/storage/init.go +++ b/storage/init.go @@ -31,6 +31,7 @@ import ( "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/nbs" "github.com/dolthub/dolt/go/store/types" ) @@ -83,6 +84,44 @@ func InitStore(ctx context.Context, absPath, ownerName, ownerEmail string) (err return nil } +// InitEmptyStore creates a genuinely empty bare NBS chunk store at absPath — +// a directory whose store root is the empty hash, with NO commits and NO +// working set. Unlike InitStore it deliberately does NOT call WriteEmptyRepo: +// an initial "Initialize data repository" commit would make the first push to +// this store a non-fast-forward and be rejected. An empty store lets the +// client's first push land as the initial history. Used by push-to-create. +// absPath must be absolute; on any failure the directory is removed. +func InitEmptyStore(ctx context.Context, absPath string) (err error) { + if !filepath.IsAbs(absPath) { + return fmt.Errorf("storage: InitEmptyStore 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) + } + }() + + // Opening a plain (non-generational) NBS store over the freshly created, + // empty directory both validates the directory (checkDir requires it to + // exist) and confirms the store is a valid empty store (a missing manifest + // is treated lazily as an empty store with the null root). This mirrors the + // construction storage.Cache.Get uses to serve pushes; a plain store closes + // cleanly, unlike the generational store LoadDoltDB routes through. + cs, err := nbs.NewLocalStore(ctx, types.Format_DOLT.VersionString(), absPath, defaultMemTableSize, nbs.NewUnlimitedMemQuotaProvider(), false) + if err != nil { + return fmt.Errorf("storage: open empty store at %q: %w", absPath, err) + } + if err = cs.Close(); err != nil { + return fmt.Errorf("storage: close empty store 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. diff --git a/storage/initempty_test.go b/storage/initempty_test.go new file mode 100644 index 0000000000000000000000000000000000000000..587491ab73bd69a3ddaefc94ff213f83e880bf5e --- /dev/null +++ b/storage/initempty_test.go @@ -0,0 +1,48 @@ +package storage + +import ( + "context" + "os" + "testing" + + "github.com/dolthub/dolt/go/store/hash" + "github.com/dolthub/dolt/go/store/nbs" + "github.com/dolthub/dolt/go/store/types" +) + +// TestInitEmptyStore verifies that InitEmptyStore creates a directory whose NBS +// store root is the empty hash (no initial commit), so a client's first push +// lands as fast-forward-from-empty. +func TestInitEmptyStore(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + absPath := RepoDiskPath(root, "alice", "empty") + + if err := InitEmptyStore(ctx, absPath); err != nil { + t.Fatalf("InitEmptyStore: %v", err) + } + + info, err := os.Stat(absPath) + if err != nil { + t.Fatalf("stat store dir: %v", err) + } + if !info.IsDir() { + t.Fatalf("expected %q to be a directory", absPath) + } + + // Reopen via the same construction Cache.Get uses and confirm the store has + // no root (the empty hash) — i.e. no commits were written. + cs, err := nbs.NewLocalStore(ctx, types.Format_DOLT.VersionString(), absPath, defaultMemTableSize, nbs.NewUnlimitedMemQuotaProvider(), false) + if err != nil { + t.Fatalf("reopen store: %v", err) + } + defer cs.Close() + + rootHash, err := cs.Root(ctx) + if err != nil { + t.Fatalf("Root: %v", err) + } + if rootHash != (hash.Hash{}) { + t.Fatalf("expected empty root hash, got %s", rootHash.String()) + } +}