diff --git a/spindle/ingester.go b/spindle/ingester.go --- a/spindle/ingester.go +++ b/spindle/ingester.go @@ -5,6 +5,7 @@ "encoding/json" "errors" "fmt" + "strings" "time" "tangled.org/core/api/tangled" @@ -154,26 +155,27 @@ } domain := s.cfg.Server.Hostname + rkey := e.Commit.RKey // no spindle configured for this repo if record.Spindle == nil { - l.Info("no spindle configured", "name", record.Name) + l.Info("no spindle configured", "rkey", rkey) return nil } // this repo did not want this spindle if *record.Spindle != domain { - l.Info("different spindle configured", "name", record.Name, "spindle", *record.Spindle, "domain", domain) + l.Info("different spindle configured", "rkey", rkey, "spindle", *record.Spindle, "domain", domain) return nil } // add this repo to the watch list - if err := s.db.AddRepo(record.Knot, did, record.Name); err != nil { + if err := s.db.AddRepo(record.Knot, did, rkey); err != nil { l.Error("failed to add repo", "error", err) return fmt.Errorf("failed to add repo: %w", err) } - didSlashRepo, err := securejoin.SecureJoin(did, record.Name) + didSlashRepo, err := securejoin.SecureJoin(did, rkey) if err != nil { return err } @@ -228,10 +230,18 @@ var rbacResource string var ownerDid string switch { - case record.Repo != nil: - repoAt, parseErr := syntax.ParseATURI(*record.Repo) + case strings.HasPrefix(record.Repo, "did:"): + resolvedOwner, repoName, lookupErr := s.resolveRepoDid(ctx, e.Did, record.Repo) + if lookupErr != nil { + return fmt.Errorf("unknown repo DID %s: %w", record.Repo, lookupErr) + } + ownerDid = resolvedOwner + rbacResource, _ = securejoin.SecureJoin(ownerDid, repoName) + + case strings.Contains(record.Repo, "/"): + repoAt, parseErr := syntax.ParseATURI(record.Repo) if parseErr != nil { - l.Info("rejecting record, invalid repoAt", "repoAt", *record.Repo) + l.Info("rejecting record, invalid repoAt", "repoAt", record.Repo) return nil } @@ -249,12 +259,14 @@ return getErr } - repo := resp.Value.Val.(*tangled.Repo) - rbacResource, _ = securejoin.SecureJoin(owner.DID.String(), repo.Name) + if _, ok := resp.Value.Val.(*tangled.Repo); !ok { + return fmt.Errorf("record at %s is not a tangled.Repo", repoAt) + } + rbacResource, _ = securejoin.SecureJoin(owner.DID.String(), repoAt.RecordKey().String()) ownerDid = owner.DID.String() default: - l.Info("rejecting collaborator record without repo at-uri (spindle RBAC keyed by owner/name)") + l.Info("rejecting collaborator record with unrecognized repo format", "repo", record.Repo) return nil } @@ -270,6 +282,46 @@ return nil } return nil +} + +func (s *Spindle) resolveRepoDid(ctx context.Context, ownerDid string, repoDid string) (string, string, error) { + owner, resolveErr := s.res.ResolveIdent(ctx, ownerDid) + if resolveErr != nil || owner.Handle.IsInvalidHandle() { + return "", "", fmt.Errorf("failed to resolve owner %s: %w", ownerDid, resolveErr) + } + + xrpcc := xrpc.Client{ + Host: owner.PDSEndpoint(), + } + + cursor := "" + for { + resp, listErr := comatproto.RepoListRecords(ctx, &xrpcc, tangled.RepoNSID, cursor, 100, ownerDid, false) + if listErr != nil { + return "", "", fmt.Errorf("failed to list repo records for %s: %w", ownerDid, listErr) + } + + for _, r := range resp.Records { + if r == nil { + continue + } + repo, ok := r.Value.Val.(*tangled.Repo) + if !ok { + continue + } + if repo.RepoDid != nil && *repo.RepoDid == repoDid { + rkey := r.Uri[strings.LastIndex(r.Uri, "/")+1:] + return ownerDid, rkey, nil + } + } + + if resp.Cursor == nil || *resp.Cursor == "" { + break + } + cursor = *resp.Cursor + } + + return "", "", fmt.Errorf("repo DID %s not found in records for %s", repoDid, ownerDid) } func (s *Spindle) fetchAndAddCollaborators(ctx context.Context, owner *identity.Identity, didSlashRepo string) error { diff --git a/appview/db/repos_rename_test.go b/appview/db/repos_rename_test.go new file mode 100644 --- /dev/null +++ b/appview/db/repos_rename_test.go @@ -0,0 +1,279 @@ +package db + +import ( + "context" + "database/sql" + "errors" + "path/filepath" + "testing" + + "tangled.org/core/appview/models" + "tangled.org/core/orm" +) + +func newTestDB(t *testing.T) *DB { + t.Helper() + path := filepath.Join(t.TempDir(), "test.db") + d, err := Make(context.Background(), path) + if err != nil { + t.Fatalf("Make: %v", err) + } + t.Cleanup(func() { d.Close() }) + return d +} + +func seedRepo(t *testing.T, d *DB, did, knot, name, rkey, repoDid string) *models.Repo { + t.Helper() + tx, err := d.Begin() + if err != nil { + t.Fatalf("Begin: %v", err) + } + repo := &models.Repo{ + Did: did, + Name: name, + Knot: knot, + Rkey: rkey, + RepoDid: repoDid, + } + if err := AddRepo(tx, repo); err != nil { + t.Fatalf("AddRepo: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("Commit: %v", err) + } + return repo +} + +func TestRenameRepo_HappyPath(t *testing.T) { + d := newTestDB(t) + seedRepo(t, d, "did:plc:akshay", "knot.example", "foo", "foo", "did:plc:repo1") + + tx, err := d.Begin() + if err != nil { + t.Fatalf("Begin: %v", err) + } + defer tx.Rollback() + + if err := RenameRepo(tx, "did:plc:akshay", "foo", "bar", "Bar"); err != nil { + t.Fatalf("RenameRepo: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("Commit: %v", err) + } + + got, err := GetRepoByDid(d, "did:plc:repo1") + if err != nil { + t.Fatalf("GetRepoByDid: %v", err) + } + if got.Rkey != "bar" { + t.Errorf("rkey = %q, want %q", got.Rkey, "bar") + } + if got.Name != "Bar" { + t.Errorf("name = %q, want %q", got.Name, "Bar") + } +} + +func TestUpdateRepoDisplayName_HappyPath(t *testing.T) { + d := newTestDB(t) + seedRepo(t, d, "did:plc:akshay", "knot.example", "foo", "foo", "did:plc:repo1") + + if err := UpdateRepoDisplayName(d, "did:plc:akshay", "foo", "Foo"); err != nil { + t.Fatalf("UpdateRepoDisplayName: %v", err) + } + + got, err := GetRepoByDid(d, "did:plc:repo1") + if err != nil { + t.Fatalf("GetRepoByDid: %v", err) + } + if got.Name != "Foo" { + t.Errorf("name = %q, want %q", got.Name, "Foo") + } + if got.Rkey != "foo" { + t.Errorf("rkey should be unchanged but got %q, want %q", got.Rkey, "foo") + } +} + +func TestRecordAndLookupRepoRename(t *testing.T) { + d := newTestDB(t) + seedRepo(t, d, "did:plc:akshay", "knot.example", "bar", "rkey1", "did:plc:repo1") + + if err := RecordRepoRename(d, "did:plc:akshay", "foo", "did:plc:repo1"); err != nil { + t.Fatalf("RecordRepoRename: %v", err) + } + + repo, err := LookupRepoRename(d, "did:plc:akshay", "foo") + if err != nil { + t.Fatalf("LookupRepoRename: %v", err) + } + if repo.RepoDid != "did:plc:repo1" { + t.Errorf("repoDid = %q, want %q", repo.RepoDid, "did:plc:repo1") + } + if repo.Name != "bar" { + t.Errorf("name = %q, want %q", repo.Name, "bar") + } +} + +func TestLookupRepoRename_MultipleOldNamesResolveToCurrent(t *testing.T) { + d := newTestDB(t) + seedRepo(t, d, "did:plc:akshay", "knot.example", "baz", "baz", "did:plc:repo1") + + if err := RecordRepoRename(d, "did:plc:akshay", "foo", "did:plc:repo1"); err != nil { + t.Fatalf("record foo: %v", err) + } + if err := RecordRepoRename(d, "did:plc:akshay", "bar", "did:plc:repo1"); err != nil { + t.Fatalf("record bar: %v", err) + } + + for _, oldName := range []string{"foo", "bar"} { + repo, err := LookupRepoRename(d, "did:plc:akshay", oldName) + if err != nil { + t.Fatalf("lookup %q: %v", oldName, err) + } + if repo.Name != "baz" { + t.Errorf("lookup %q: name = %q, want %q", oldName, repo.Name, "baz") + } + } +} + +func TestRecordRepoRename_UpsertRefreshesTarget(t *testing.T) { + d := newTestDB(t) + seedRepo(t, d, "did:plc:akshay", "knot.example", "current", "rkey1", "did:plc:repo1") + seedRepo(t, d, "did:plc:akshay", "knot.example", "other", "rkey2", "did:plc:repo2") + + if err := RecordRepoRename(d, "did:plc:akshay", "shared", "did:plc:repo1"); err != nil { + t.Fatalf("first record: %v", err) + } + if err := RecordRepoRename(d, "did:plc:akshay", "shared", "did:plc:repo2"); err != nil { + t.Fatalf("second record: %v", err) + } + + repo, err := LookupRepoRename(d, "did:plc:akshay", "shared") + if err != nil { + t.Fatalf("LookupRepoRename: %v", err) + } + if repo.RepoDid != "did:plc:repo2" { + t.Errorf("latest record should win: repoDid = %q, want %q", repo.RepoDid, "did:plc:repo2") + } +} + +func TestLookupRepoRename_StaleSelfHeal(t *testing.T) { + d := newTestDB(t) + + if err := RecordRepoRename(d, "did:plc:akshay", "foo", "did:plc:ghost"); err != nil { + t.Fatalf("RecordRepoRename: %v", err) + } + + _, err := LookupRepoRename(d, "did:plc:akshay", "foo") + if !errors.Is(err, sql.ErrNoRows) { + t.Errorf("target should be gone and fall through to 404: err = %v, want sql.ErrNoRows", err) + } +} + +func TestLookupRepoRename_NoRow(t *testing.T) { + d := newTestDB(t) + + _, err := LookupRepoRename(d, "did:plc:akshay", "nothing") + if !errors.Is(err, sql.ErrNoRows) { + t.Errorf("err = %v, want sql.ErrNoRows", err) + } +} + +func TestDuplicateRkeyUnderSameDID_Rejected(t *testing.T) { + d := newTestDB(t) + seedRepo(t, d, "did:plc:akshay", "knot.example", "myrepo", "myrepo", "did:plc:repo1") + + tx, err := d.Begin() + if err != nil { + t.Fatalf("Begin: %v", err) + } + defer tx.Rollback() + + err = AddRepo(tx, &models.Repo{ + Did: "did:plc:akshay", + Name: "myrepo", + Knot: "knot.example", + Rkey: "myrepo", + RepoDid: "did:plc:repo2", + }) + if err == nil { + t.Fatal("expected unique violation for duplicate (did, rkey), got nil") + } + if !orm.IsUniqueViolation(err) { + t.Errorf("err = %v, want unique violation", err) + } +} + +func TestRenameRepo_OldRkeyRowGone(t *testing.T) { + d := newTestDB(t) + seedRepo(t, d, "did:plc:akshay", "knot.example", "old", "old", "did:plc:repo1") + + tx, err := d.Begin() + if err != nil { + t.Fatalf("Begin: %v", err) + } + defer tx.Rollback() + + if err := RenameRepo(tx, "did:plc:akshay", "old", "new", "New"); err != nil { + t.Fatalf("RenameRepo: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("Commit: %v", err) + } + + got, err := GetRepoByDid(d, "did:plc:repo1") + if err != nil { + t.Fatalf("GetRepoByDid: %v", err) + } + if got.Rkey != "new" { + t.Errorf("rkey = %q, want %q", got.Rkey, "new") + } + + var dummy int + err = d.QueryRow(`select 1 from repos where did = ? and rkey = ?`, "did:plc:akshay", "old").Scan(&dummy) + if !errors.Is(err, sql.ErrNoRows) { + t.Errorf("old rkey row should be gone, got err = %v", err) + } +} + +func TestRenameRepo_PipelineRenamed(t *testing.T) { + d := newTestDB(t) + seedRepo(t, d, "did:plc:akshay", "knot.example", "old", "old", "did:plc:repo1") + + if _, err := d.Exec( + `insert into triggers (kind) values (?)`, "push", + ); err != nil { + t.Fatalf("seed trigger: %v", err) + } + if _, err := d.Exec( + `insert into pipelines (rkey, knot, repo_owner, repo_name, sha, trigger_id, repo_did) + values (?, ?, ?, ?, ?, ?, ?)`, + "pipe1", "knot.example", "did:plc:akshay", "old", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", 1, "did:plc:repo1", + ); err != nil { + t.Fatalf("seed pipeline: %v", err) + } + + tx, err := d.Begin() + if err != nil { + t.Fatalf("Begin: %v", err) + } + defer tx.Rollback() + + if err := RenameRepo(tx, "did:plc:akshay", "old", "new", "New"); err != nil { + t.Fatalf("RenameRepo: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("Commit: %v", err) + } + + var repoName string + if err := d.QueryRow( + `select repo_name from pipelines where repo_owner = ? and rkey = ?`, + "did:plc:akshay", "pipe1", + ).Scan(&repoName); err != nil { + t.Fatalf("query pipeline: %v", err) + } + if repoName != "new" { + t.Errorf("pipeline repo_name = %q, want %q", repoName, "new") + } +} diff --git a/appview/repo/rename_test.go b/appview/repo/rename_test.go new file mode 100644 --- /dev/null +++ b/appview/repo/rename_test.go @@ -0,0 +1,53 @@ +package repo + +import ( + "strings" + "testing" +) + +func TestValidateRenameInput(t *testing.T) { + const validTID = "3jzfcijpj2z2a" + cases := []struct { + name string + currentName string + currentRkey string + raw string + wantName string + wantErrSub string + }{ + {"happy path", "foo", "", "bar", "bar", ""}, + {"trims surrounding whitespace", "foo", "", " bar ", "bar", ""}, + {"strips .git suffix", "foo", "", "bar.git", "bar", ""}, + {"empty after trim", "foo", "", " ", "", "cannot be empty"}, + {"raw empty", "foo", "", "", "", "cannot be empty"}, + {"path traversal slash", "foo", "", "../bar", "", "invalid path"}, + {"invalid character", "foo", "", "ba r", "", "alphanumeric"}, + {"same name as current with non-TID rkey", "foo", "foo", "foo", "", "matches the current name"}, + {"same name as current with TID rkey allowed", "foo", validTID, "foo", "foo", ""}, + {"case-only diff is not a no-op", "foo", "", "Foo", "Foo", ""}, + {"strip-git collides with current", "foo", "", "foo.git", "", "matches the current name"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := validateRenameInput(tc.currentName, tc.currentRkey, tc.raw) + if tc.wantErrSub == "" { + if err != nil { + t.Fatalf("err = %v, want nil", err) + } + if got != tc.wantName { + t.Errorf("name = %q, want %q", got, tc.wantName) + } + return + } + if err == nil { + t.Fatalf("err = nil, want error containing %q", tc.wantErrSub) + } + if got != "" { + t.Errorf("name = %q, want empty on error", got) + } + if !strings.Contains(err.Error(), tc.wantErrSub) { + t.Errorf("err = %q, want substring %q", err.Error(), tc.wantErrSub) + } + }) + } +} diff --git a/spindle/xrpc/add_secret.go b/spindle/xrpc/add_secret.go --- a/spindle/xrpc/add_secret.go +++ b/spindle/xrpc/add_secret.go @@ -61,8 +61,11 @@ return } - repo := resp.Value.Val.(*tangled.Repo) - didPath, err := securejoin.SecureJoin(ident.DID.String(), repo.Name) + if _, ok := resp.Value.Val.(*tangled.Repo); !ok { + fail(xrpcerr.RepoNotFoundError) + return + } + didPath, err := securejoin.SecureJoin(ident.DID.String(), repoAt.RecordKey().String()) if err != nil { fail(xrpcerr.GenericError(err)) return diff --git a/spindle/xrpc/list_secrets.go b/spindle/xrpc/list_secrets.go --- a/spindle/xrpc/list_secrets.go +++ b/spindle/xrpc/list_secrets.go @@ -56,8 +56,11 @@ return } - repo := resp.Value.Val.(*tangled.Repo) - didPath, err := securejoin.SecureJoin(ident.DID.String(), repo.Name) + if _, ok := resp.Value.Val.(*tangled.Repo); !ok { + fail(xrpcerr.RepoNotFoundError) + return + } + didPath, err := securejoin.SecureJoin(ident.DID.String(), repoAt.RecordKey().String()) if err != nil { fail(xrpcerr.GenericError(err)) return diff --git a/spindle/xrpc/pipeline_cancel_pipeline.go b/spindle/xrpc/pipeline_cancel_pipeline.go --- a/spindle/xrpc/pipeline_cancel_pipeline.go +++ b/spindle/xrpc/pipeline_cancel_pipeline.go @@ -66,8 +66,11 @@ return } - repo := resp.Value.Val.(*tangled.Repo) - didSlashRepo, err := securejoin.SecureJoin(ident.DID.String(), repo.Name) + if _, ok := resp.Value.Val.(*tangled.Repo); !ok { + fail(xrpcerr.RepoNotFoundError) + return + } + didSlashRepo, err := securejoin.SecureJoin(ident.DID.String(), repoAt.RecordKey().String()) if err != nil { fail(xrpcerr.GenericError(err)) return diff --git a/spindle/xrpc/remove_secret.go b/spindle/xrpc/remove_secret.go --- a/spindle/xrpc/remove_secret.go +++ b/spindle/xrpc/remove_secret.go @@ -55,8 +55,11 @@ return } - repo := resp.Value.Val.(*tangled.Repo) - didPath, err := securejoin.SecureJoin(ident.DID.String(), repo.Name) + if _, ok := resp.Value.Val.(*tangled.Repo); !ok { + fail(xrpcerr.RepoNotFoundError) + return + } + didPath, err := securejoin.SecureJoin(ident.DID.String(), repoAt.RecordKey().String()) if err != nil { fail(xrpcerr.GenericError(err)) return