diff --git a/appview/ingester.go b/appview/ingester.go --- a/appview/ingester.go +++ b/appview/ingester.go @@ -29,6 +29,7 @@ "tangled.org/core/appview/db" "tangled.org/core/appview/models" "tangled.org/core/appview/notify" + "tangled.org/core/appview/repoverify" "tangled.org/core/appview/serververify" "tangled.org/core/appview/validator" "tangled.org/core/idresolver" @@ -45,6 +46,7 @@ Logger *slog.Logger Validator *validator.Validator Notifier notify.Notifier + Verifier repoverify.Verifier } type processFunc func(ctx context.Context, e *jmodels.Event) error @@ -1047,8 +1049,8 @@ return fmt.Errorf("failed to validate issue: %w", err) } - if record.Repo != nil { - repo, repoErr := db.GetRepoByAtUri(i.Db, *record.Repo) + if record.Repo != "" && !strings.HasPrefix(record.Repo, "did:") { + repo, repoErr := db.GetRepoByAtUri(i.Db, record.Repo) if repoErr == nil && repo.RepoDid != "" { if enqErr := db.EnqueuePdsRecordMigration(ctx, i.Db, "add-repo-did", syntax.DID(did), syntax.NSID(tangled.RepoIssueNSID), syntax.RecordKey(e.Commit.RKey)); enqErr != nil { l.Warn("failed to enqueue PDS rewrite for issue", "err", enqErr, "did", did, "repoDid", repo.RepoDid) diff --git a/appview/ingester_repo.go b/appview/ingester_repo.go --- a/appview/ingester_repo.go +++ b/appview/ingester_repo.go @@ -6,13 +6,16 @@ "encoding/json" "errors" "fmt" + "log/slog" "slices" + "strings" "github.com/bluesky-social/indigo/atproto/syntax" jmodels "github.com/bluesky-social/jetstream/pkg/models" "tangled.org/core/api/tangled" "tangled.org/core/appview/db" "tangled.org/core/appview/models" + "tangled.org/core/appview/repoverify" "tangled.org/core/orm" ) @@ -47,7 +50,15 @@ } repoDid := *record.RepoDid - _, err := db.GetRepo(i.Db, + proceed, err := i.verifyOwnership(ctx, l, repoDid, e.Did, record.Knot) + if err != nil { + return err + } + if !proceed { + return nil + } + + _, err = db.GetRepo(i.Db, orm.FilterEq("did", e.Did), orm.FilterEq("rkey", e.Commit.RKey), ) @@ -165,6 +176,14 @@ return nil } + proceed, err := i.verifyOwnership(ctx, l, *record.RepoDid, e.Did, record.Knot) + if err != nil { + return err + } + if !proceed { + return nil + } + current, err := db.GetRepo(i.Db, orm.FilterEq("did", e.Did), orm.FilterEq("rkey", e.Commit.RKey), @@ -175,6 +194,14 @@ return nil } return fmt.Errorf("failed to fetch repo for ingest: %w", err) + } + + if current.RepoDid != "" && current.RepoDid != *record.RepoDid { + l.Warn("rejecting repo update: repoDid is immutable", + "currentRepoDid", current.RepoDid, + "recordRepoDid", *record.RepoDid, + ) + return nil } desired := repoFromRecord(current, &record) @@ -329,4 +356,37 @@ return "" } return *s +} + +func (i *Ingester) verifyOwnership(ctx context.Context, l *slog.Logger, repoDid, eventDid, recordKnot string) (bool, error) { + if i.Verifier == nil { + return false, fmt.Errorf("ingester has no repo ownership verifier configured") + } + rd, err := repoverify.NewRepoDid(repoDid) + if err != nil { + l.Warn("rejecting repo event: invalid repoDid on record", "repoDid", repoDid, "err", err) + return false, nil + } + result, err := i.Verifier(ctx, rd) + if err != nil { + return false, fmt.Errorf("verify repo ownership: %w", err) + } + if result.OwnerDid.String() != eventDid { + l.Warn("rejecting repo event: owner mismatch", + "repoDid", repoDid, + "claimedOwner", eventDid, + "knotOwner", result.OwnerDid.String(), + "knot", result.KnotURL.String(), + ) + return false, nil + } + if !strings.EqualFold(recordKnot, result.KnotURL.Host) { + l.Warn("rejecting repo event: record knot does not match DID-doc endpoint", + "repoDid", repoDid, + "recordKnot", recordKnot, + "canonicalKnot", result.KnotURL.Host, + ) + return false, nil + } + return true, nil } diff --git a/appview/ingester_repo_test.go b/appview/ingester_repo_test.go --- a/appview/ingester_repo_test.go +++ b/appview/ingester_repo_test.go @@ -7,6 +7,7 @@ "errors" "io" "log/slog" + "net/url" "path/filepath" "testing" @@ -16,8 +17,36 @@ "tangled.org/core/appview/db" "tangled.org/core/appview/models" "tangled.org/core/appview/notify" + "tangled.org/core/appview/repoverify" "tangled.org/core/orm" ) + +func mustKnotURL(t *testing.T, raw string) *url.URL { + t.Helper() + u, err := repoverify.ParseKnotEndpoint(raw, true) + if err != nil { + t.Fatalf("ParseKnotEndpoint(%q): %v", raw, err) + } + return u +} + +func acceptOwner(t *testing.T, e *jmodels.Event) repoverify.Verifier { + t.Helper() + knot := mustKnotURL(t, "https://knot.example") + return func(_ context.Context, repoDid repoverify.RepoDid) (repoverify.Result, error) { + return repoverify.Result{ + RepoDid: repoDid, + OwnerDid: repoverify.OwnerDid(e.Did), + KnotURL: knot, + }, nil + } +} + +func stubVerifier(result repoverify.Result, err error) repoverify.Verifier { + return func(_ context.Context, _ repoverify.RepoDid) (repoverify.Result, error) { + return result, err + } +} type spyNotifier struct { notify.BaseNotifier @@ -48,6 +77,17 @@ Notifier: spy, } return ing, spy +} + +func withVerifier(ing *Ingester, v repoverify.Verifier) *Ingester { + ing.Verifier = v + return ing +} + +func ingestAcceptingOwner(t *testing.T, ing *Ingester, e *jmodels.Event) error { + t.Helper() + ing.Verifier = acceptOwner(t, e) + return ing.ingestRepo(context.Background(), e) } func seedRepoRow(t *testing.T, ing *Ingester, did, knot, name, rkey, repoDid string) *models.Repo { @@ -126,7 +166,7 @@ RepoDid: ptr("did:plc:repo1"), }) - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } @@ -155,7 +195,7 @@ RepoDid: ptr("did:plc:repo1"), }) - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } if spy.creates != 0 { @@ -173,7 +213,7 @@ RepoDid: ptr("did:plc:repo1"), }) - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } @@ -217,7 +257,7 @@ Name: ptr("myrepo"), }) - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } if spy.creates != 0 { @@ -238,7 +278,7 @@ RepoDid: ptr("did:plc:repo1"), }) - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } @@ -264,7 +304,7 @@ RepoDid: ptr("did:plc:repo1"), }) - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } @@ -287,7 +327,7 @@ RepoDid: ptr("did:plc:repo1"), }) - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } @@ -315,7 +355,7 @@ e = makeDeleteEvent("did:plc:nobody", "ghost") } - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } }) @@ -331,7 +371,7 @@ Name: ptr("bar"), }) - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } @@ -346,7 +386,7 @@ seedRepoRow(t, ing, "did:plc:akshay", "knot.example", "foo", "foo", "did:plc:repo1") e := makeDeleteEvent("did:plc:akshay", "foo") - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } @@ -373,7 +413,7 @@ }, } - if err := ing.ingestRepo(context.Background(), e); err == nil { + if err := ingestAcceptingOwner(t, ing, e); err == nil { t.Errorf("ingestRepo with malformed record: err = nil, want error") } } @@ -394,12 +434,12 @@ Name: ptr("NewName"), RepoDid: ptr("did:plc:repo1"), }) - if err := ing.ingestRepo(context.Background(), createEvt); err != nil { + if err := ingestAcceptingOwner(t, ing, createEvt); err != nil { t.Fatalf("ingest create: %v", err) } deleteEvt := makeDeleteEvent("did:plc:akshay", "oldname") - if err := ing.ingestRepo(context.Background(), deleteEvt); err != nil { + if err := ingestAcceptingOwner(t, ing, deleteEvt); err != nil { t.Fatalf("ingest delete: %v", err) } @@ -446,12 +486,269 @@ RepoDid: ptr("did:plc:repo1"), }) - if err := ing.ingestRepo(context.Background(), e); err != nil { + if err := ingestAcceptingOwner(t, ing, e); err != nil { t.Fatalf("ingestRepo: %v", err) } r := loadRepo(t, ing, "did:plc:akshay", "myrepo") if r.Name != "myrepo" { t.Errorf("name should fall back to rkey: got %q, want %q", r.Name, "myrepo") + } +} + +func TestIngestRepo_CreateSquatRejected(t *testing.T) { + ing, spy := newTestIngester(t) + + e := makeEvent(t, jmodels.CommitOperationCreate, "did:plc:boltless", "squatrepo", tangled.Repo{ + Knot: "knot.example", + RepoDid: ptr("did:plc:akshays-repo"), + }) + + withVerifier(ing, stubVerifier(repoverify.Result{ + RepoDid: "did:plc:akshays-repo", + OwnerDid: "did:plc:akshay", + KnotURL: mustKnotURL(t, "https://knot.example"), + }, nil)) + + if err := ing.ingestRepo(context.Background(), e); err != nil { + t.Fatalf("ingestRepo: %v", err) + } + + if _, err := db.GetRepo(ing.Db, + orm.FilterEq("did", "did:plc:boltless"), + orm.FilterEq("rkey", "squatrepo"), + ); !errors.Is(err, sql.ErrNoRows) { + t.Fatalf("boltless's squat row should not exist, got err=%v", err) + } + if spy.creates != 0 { + t.Errorf("NewRepo called %d times despite rejection", spy.creates) + } +} + +func TestIngestRepo_CreateHijackExistingRepoRejected(t *testing.T) { + ing, spy := newTestIngester(t) + seedRepoRow(t, ing, "did:plc:akshay", "knot.example", "myrepo", "akshayskey", "did:plc:akshays-repo") + + e := makeEvent(t, jmodels.CommitOperationCreate, "did:plc:boltless", "takeover", tangled.Repo{ + Knot: "knot.example", + RepoDid: ptr("did:plc:akshays-repo"), + }) + + withVerifier(ing, stubVerifier(repoverify.Result{ + RepoDid: "did:plc:akshays-repo", + OwnerDid: "did:plc:akshay", + KnotURL: mustKnotURL(t, "https://knot.example"), + }, nil)) + + if err := ing.ingestRepo(context.Background(), e); err != nil { + t.Fatalf("ingestRepo: %v", err) + } + + akshay := loadRepo(t, ing, "did:plc:akshay", "akshayskey") + if akshay.Did != "did:plc:akshay" || akshay.Rkey != "akshayskey" { + t.Errorf("akshay's row mutated: %+v", akshay) + } + if spy.renames != 0 { + t.Errorf("RenameRepo called %d times despite rejection", spy.renames) + } +} + +func TestIngestRepo_CreateRenameIgnoresRkeyDrift(t *testing.T) { + ing, spy := newTestIngester(t) + seedRepoRow(t, ing, "did:plc:akshay", "knot.example", "oldname", "oldrkey", "did:plc:akshays-repo") + + e := makeEvent(t, jmodels.CommitOperationCreate, "did:plc:akshay", "newrkey", tangled.Repo{ + Knot: "knot.example", + Name: ptr("newname"), + RepoDid: ptr("did:plc:akshays-repo"), + }) + + withVerifier(ing, stubVerifier(repoverify.Result{ + RepoDid: "did:plc:akshays-repo", + OwnerDid: "did:plc:akshay", + KnotURL: mustKnotURL(t, "https://knot.example"), + }, nil)) + + if err := ing.ingestRepo(context.Background(), e); err != nil { + t.Fatalf("ingestRepo: %v", err) + } + + r := loadRepo(t, ing, "did:plc:akshay", "newrkey") + if r.Name != "newname" { + t.Errorf("rename did not apply despite matching owner: name=%q", r.Name) + } + if spy.renames != 1 { + t.Errorf("RenameRepo called %d times, want 1", spy.renames) + } +} + +func TestIngestRepo_CreateVerifierTransientErrorPropagates(t *testing.T) { + ing, spy := newTestIngester(t) + + e := makeEvent(t, jmodels.CommitOperationCreate, "did:plc:akshay", "myrepo", tangled.Repo{ + Knot: "knot.example", + RepoDid: ptr("did:plc:akshays-repo"), + }) + + withVerifier(ing, stubVerifier(repoverify.Result{}, errors.New("knot unreachable"))) + + err := ing.ingestRepo(context.Background(), e) + if err == nil { + t.Fatalf("expected error on transient verifier failure, got nil") + } + if spy.creates != 0 { + t.Errorf("NewRepo called %d times despite verifier error", spy.creates) + } +} + +func TestIngestRepo_UpdateRejectsOwnerMismatch(t *testing.T) { + ing, _ := newTestIngester(t) + seedRepoRow(t, ing, "did:plc:akshay", "knot.example", "myrepo", "akshayskey", "did:plc:akshays-repo") + + e := makeEvent(t, jmodels.CommitOperationUpdate, "did:plc:boltless", "akshayskey", tangled.Repo{ + Knot: "knot.example", + Description: ptr("boltless hijacks metadata"), + RepoDid: ptr("did:plc:akshays-repo"), + }) + + withVerifier(ing, stubVerifier(repoverify.Result{ + RepoDid: "did:plc:akshays-repo", + OwnerDid: "did:plc:akshay", + KnotURL: mustKnotURL(t, "https://knot.example"), + }, nil)) + + if err := ing.ingestRepo(context.Background(), e); err != nil { + t.Fatalf("ingestRepo: %v", err) + } + + akshay := loadRepo(t, ing, "did:plc:akshay", "akshayskey") + if akshay.Description == "boltless hijacks metadata" { + t.Errorf("update by non-owner applied: %+v", akshay) + } +} + +func TestIngestRepo_CreateInvalidRepoDidRejected(t *testing.T) { + ing, spy := newTestIngester(t) + + e := makeEvent(t, jmodels.CommitOperationCreate, "did:plc:akshay", "myrepo", tangled.Repo{ + Knot: "knot.example", + RepoDid: ptr("did:plc:"), + }) + + verifierCalled := false + withVerifier(ing, func(_ context.Context, _ repoverify.RepoDid) (repoverify.Result, error) { + verifierCalled = true + return repoverify.Result{}, nil + }) + + if err := ing.ingestRepo(context.Background(), e); err != nil { + t.Fatalf("ingestRepo: %v", err) + } + if verifierCalled { + t.Errorf("verifier was called with an invalid repoDid") + } + if spy.creates != 0 { + t.Errorf("NewRepo called %d times despite invalid repoDid", spy.creates) + } +} + +func TestIngestRepo_NilVerifierFailsClosed(t *testing.T) { + ing, spy := newTestIngester(t) + + e := makeEvent(t, jmodels.CommitOperationCreate, "did:plc:akshay", "myrepo", tangled.Repo{ + Knot: "knot.example", + RepoDid: ptr("did:plc:akshays-repo"), + }) + + err := ing.ingestRepo(context.Background(), e) + if err == nil { + t.Fatalf("expected error when Verifier is nil, got nil") + } + if spy.creates != 0 { + t.Errorf("NewRepo called %d times despite nil verifier", spy.creates) + } +} + +func TestIngestRepo_CreateRejectsKnotMismatch(t *testing.T) { + ing, spy := newTestIngester(t) + + e := makeEvent(t, jmodels.CommitOperationCreate, "did:plc:akshay", "myrepo", tangled.Repo{ + Knot: "evil.example", + RepoDid: ptr("did:plc:akshays-repo"), + }) + + withVerifier(ing, stubVerifier(repoverify.Result{ + RepoDid: "did:plc:akshays-repo", + OwnerDid: "did:plc:akshay", + KnotURL: mustKnotURL(t, "https://knot.example"), + }, nil)) + + if err := ing.ingestRepo(context.Background(), e); err != nil { + t.Fatalf("ingestRepo: %v", err) + } + if _, err := db.GetRepo(ing.Db, + orm.FilterEq("did", "did:plc:akshay"), + orm.FilterEq("rkey", "myrepo"), + ); !errors.Is(err, sql.ErrNoRows) { + t.Fatalf("row should not be created for spoofed knot, err=%v", err) + } + if spy.creates != 0 { + t.Errorf("NewRepo called %d times despite knot mismatch", spy.creates) + } +} + +func TestIngestRepo_UpdateRejectsKnotMismatch(t *testing.T) { + ing, _ := newTestIngester(t) + seedRepoRow(t, ing, "did:plc:akshay", "knot.example", "myrepo", "akshayskey", "did:plc:akshays-repo") + + e := makeEvent(t, jmodels.CommitOperationUpdate, "did:plc:akshay", "akshayskey", tangled.Repo{ + Knot: "evil.example", + Description: ptr("redirected clone target"), + RepoDid: ptr("did:plc:akshays-repo"), + }) + + withVerifier(ing, stubVerifier(repoverify.Result{ + RepoDid: "did:plc:akshays-repo", + OwnerDid: "did:plc:akshay", + KnotURL: mustKnotURL(t, "https://knot.example"), + }, nil)) + + if err := ing.ingestRepo(context.Background(), e); err != nil { + t.Fatalf("ingestRepo: %v", err) + } + akshay := loadRepo(t, ing, "did:plc:akshay", "akshayskey") + if akshay.Description == "redirected clone target" { + t.Errorf("update with spoofed knot applied: %+v", akshay) + } + if akshay.Knot != "knot.example" { + t.Errorf("row knot mutated to %q, want knot.example", akshay.Knot) + } +} + +func TestIngestRepo_UpdateRejectsRepoDidMutation(t *testing.T) { + ing, _ := newTestIngester(t) + seedRepoRow(t, ing, "did:plc:akshay", "knot.example", "myrepo", "akshayskey", "did:plc:akshays-repo") + + e := makeEvent(t, jmodels.CommitOperationUpdate, "did:plc:akshay", "akshayskey", tangled.Repo{ + Knot: "knot.example", + Description: ptr("sneaky repoDid swap"), + RepoDid: ptr("did:plc:other-repo"), + }) + + withVerifier(ing, stubVerifier(repoverify.Result{ + RepoDid: "did:plc:other-repo", + OwnerDid: "did:plc:akshay", + KnotURL: mustKnotURL(t, "https://knot.example"), + }, nil)) + + if err := ing.ingestRepo(context.Background(), e); err != nil { + t.Fatalf("ingestRepo: %v", err) + } + akshay := loadRepo(t, ing, "did:plc:akshay", "akshayskey") + if akshay.RepoDid != "did:plc:akshays-repo" { + t.Errorf("repoDid mutated to %q, want did:plc:akshays-repo", akshay.RepoDid) + } + if akshay.Description == "sneaky repoDid swap" { + t.Errorf("metadata from repoDid-mutating update applied: %+v", akshay) } } diff --git a/api/tangled/repodescribeRepo.go b/api/tangled/repodescribeRepo.go new file mode 100644 --- /dev/null +++ b/api/tangled/repodescribeRepo.go @@ -0,0 +1,39 @@ +// Code generated by cmd/lexgen (see Makefile's lexgen); DO NOT EDIT. + +package tangled + +// schema: sh.tangled.repo.describeRepo + +import ( + "context" + + "github.com/bluesky-social/indigo/lex/util" +) + +const ( + RepoDescribeRepoNSID = "sh.tangled.repo.describeRepo" +) + +// RepoDescribeRepo_Output is the output of a sh.tangled.repo.describeRepo call. +type RepoDescribeRepo_Output struct { + // ownerDid: DID of the current owner according to the knot. + OwnerDid string `json:"ownerDid" cborgen:"ownerDid"` + RepoDid string `json:"repoDid" cborgen:"repoDid"` + // rkey: Current rkey of the sh.tangled.repo record tracked by this knot + Rkey string `json:"rkey" cborgen:"rkey"` +} + +// RepoDescribeRepo calls the XRPC method "sh.tangled.repo.describeRepo". +// +// repoDid: DID of the git repo as minted by the knot +func RepoDescribeRepo(ctx context.Context, c util.LexClient, repoDid string) (*RepoDescribeRepo_Output, error) { + var out RepoDescribeRepo_Output + + params := map[string]interface{}{} + params["repoDid"] = repoDid + if err := c.LexDo(ctx, util.Query, "", "sh.tangled.repo.describeRepo", params, nil, &out); err != nil { + return nil, err + } + + return &out, nil +} diff --git a/appview/pulls/comment.go b/appview/pulls/comment.go --- a/appview/pulls/comment.go +++ b/appview/pulls/comment.go @@ -103,7 +103,7 @@ comment := &models.PullComment{ OwnerDid: user.Did, - RepoAt: f.RepoAt().String(), + RepoDid: string(f.RepoDid), PullId: pull.PullId, Body: body, CommentAt: atResp.Uri, diff --git a/appview/pulls/create.go b/appview/pulls/create.go --- a/appview/pulls/create.go +++ b/appview/pulls/create.go @@ -98,13 +98,13 @@ repoString := strings.SplitN(forkRepo, "/", 2) forkOwnerDid := repoString[0] - repoName := repoString[1] - fork, err := db.GetForkByDid(s.db, forkOwnerDid, repoName) + forkRkey := strings.ToLower(repoString[1]) + fork, err := db.GetForkByDid(s.db, forkOwnerDid, forkRkey) if errors.Is(err, sql.ErrNoRows) { s.pages.Notice(w, "pull", "No such fork.") return } else if err != nil { - l.Error("failed to fetch fork", "err", err, "fork_owner_did", forkOwnerDid, "repo_name", repoName) + l.Error("failed to fetch fork", "err", err, "fork_owner_did", forkOwnerDid, "fork_rkey", forkRkey) s.pages.Notice(w, "pull", "Failed to fetch fork.") return } @@ -182,17 +182,10 @@ return } - forkAtUri := fork.RepoAt() - var forkDid *syntax.DID - if fork.RepoDid != "" { - forkDid = new(syntax.DID) - *forkDid = syntax.DID(fork.RepoDid) - } - + forkDid := syntax.DID(fork.RepoDid) pullSource := &models.PullSource{ Branch: sourceBranch, - RepoAt: &forkAtUri, - RepoDid: forkDid, + RepoDid: &forkDid, } s.createPullRequest(w, r, repo, userDid, title, body, targetBranch, patch, combined, sourceRev, pullSource, isStacked, stackTitles, stackBodies) @@ -284,7 +277,7 @@ Body: body, TargetBranch: targetBranch, OwnerDid: userDid.String(), - RepoAt: repo.RepoAt(), + RepoDid: syntax.DID(repo.RepoDid), Rkey: rkey, Mentions: mentions, References: references, @@ -324,7 +317,7 @@ s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") return } - pullId, err := db.NextPullId(tx, repo.RepoAt()) + pullId, err := db.NextPullId(tx, repo.RepoDid) if err != nil { s.logger.Error("failed to get pull id", "err", err) s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") @@ -502,7 +495,7 @@ Body: body, TargetBranch: targetBranch, OwnerDid: userDid.String(), - RepoAt: repo.RepoAt(), + RepoDid: syntax.DID(repo.RepoDid), Rkey: rkey, Mentions: mentions, References: references, diff --git a/appview/pulls/lifecycle.go b/appview/pulls/lifecycle.go --- a/appview/pulls/lifecycle.go +++ b/appview/pulls/lifecycle.go @@ -66,7 +66,7 @@ } err = db.ClosePulls( tx, - orm.FilterEq("repo_at", f.RepoAt()), + orm.FilterEq("repo_did", string(f.RepoDid)), orm.FilterIn("at_uri", atUris), ) if err != nil { @@ -143,7 +143,7 @@ } err = db.ReopenPulls( tx, - orm.FilterEq("repo_at", f.RepoAt()), + orm.FilterEq("repo_did", string(f.RepoDid)), orm.FilterIn("at_uri", atUris), ) if err != nil { diff --git a/appview/pulls/list.go b/appview/pulls/list.go --- a/appview/pulls/list.go +++ b/appview/pulls/list.go @@ -103,7 +103,7 @@ searchOpts := models.PullSearchOptions{ Keywords: tf.Keywords, Phrases: tf.Phrases, - RepoAt: f.RepoAt().String(), + RepoDid: f.RepoDid, State: state, AuthorDid: authorDid, Labels: labels, @@ -175,7 +175,7 @@ } } else { filters := []orm.Filter{ - orm.FilterEq("repo_at", f.RepoAt()), + orm.FilterEq("repo_did", f.RepoDid), } if state != nil { filters = append(filters, orm.FilterEq("state", *state)) @@ -195,10 +195,10 @@ for _, p := range pulls { var pullSourceRepo *models.Repo if p.PullSource != nil { - if p.PullSource.RepoAt != nil { - pullSourceRepo, err = db.GetRepoByAtUri(s.db, p.PullSource.RepoAt.String()) + if p.PullSource.RepoDid != nil { + pullSourceRepo, err = db.GetRepoByDid(s.db, string(*p.PullSource.RepoDid)) if err != nil { - l.Error("failed to get repo by at uri", "err", err, "repo_at", p.PullSource.RepoAt.String()) + l.Error("failed to get repo by did", "err", err, "repo_did", p.PullSource.RepoDid.String()) continue } else { p.PullSource.Repo = pullSourceRepo @@ -265,7 +265,7 @@ s.db, len(shas), orm.FilterEq("p.repo_owner", f.Did), - orm.FilterEq("p.repo_name", f.Name), + orm.FilterEq("p.repo_name", f.Rkey), orm.FilterEq("p.knot", f.Knot), orm.FilterIn("p.sha", shas), ) diff --git a/appview/pulls/merge.go b/appview/pulls/merge.go --- a/appview/pulls/merge.go +++ b/appview/pulls/merge.go @@ -118,7 +118,7 @@ atUris = append(atUris, p.AtUri()) p.State = models.PullMerged } - err = db.MergePulls(tx, orm.FilterEq("repo_at", f.RepoAt()), orm.FilterIn("at_uri", atUris)) + err = db.MergePulls(tx, orm.FilterEq("repo_did", string(f.RepoDid)), orm.FilterIn("at_uri", atUris)) if err != nil { l.Error("failed to update pull request status in database", "err", err) s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.") diff --git a/appview/pulls/resubmit.go b/appview/pulls/resubmit.go --- a/appview/pulls/resubmit.go +++ b/appview/pulls/resubmit.go @@ -185,9 +185,9 @@ return } - forkRepo, err := db.GetRepoByAtUri(s.db, pull.PullSource.RepoAt.String()) + forkRepo, err := db.GetRepoByDid(s.db, string(*pull.PullSource.RepoDid)) if err != nil { - l.Error("failed to get source repo", "err", err, "repo_at", pull.PullSource.RepoAt.String()) + l.Error("failed to get source repo", "err", err, "repo_did", pull.PullSource.RepoDid.String()) s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") return } @@ -480,7 +480,7 @@ continue } - err := db.AbandonPulls(tx, orm.FilterEq("repo_at", p.RepoAt), orm.FilterEq("at_uri", p.AtUri())) + err := db.AbandonPulls(tx, orm.FilterEq("repo_did", string(p.RepoDid)), orm.FilterEq("at_uri", p.AtUri())) if err != nil { l.Error("failed to delete pull", "err", err, "pull_id", p.PullId) s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") diff --git a/appview/pulls/single.go b/appview/pulls/single.go --- a/appview/pulls/single.go +++ b/appview/pulls/single.go @@ -149,7 +149,7 @@ s.db, len(shas), orm.FilterEq("p.repo_owner", f.Did), - orm.FilterEq("p.repo_name", f.Name), + orm.FilterEq("p.repo_name", f.Rkey), orm.FilterEq("p.knot", f.Knot), orm.FilterIn("p.sha", shas), ) @@ -369,15 +369,15 @@ return pages.Unknown } - var sourceRepo syntax.ATURI - if pull.PullSource.RepoAt != nil { - sourceRepo = *pull.PullSource.RepoAt + var sourceRepoDid string + if pull.PullSource.RepoDid != nil { + sourceRepoDid = string(*pull.PullSource.RepoDid) } else { - sourceRepo = repo.RepoAt() + sourceRepoDid = repo.RepoDid } xrpcc := &indigoxrpc.Client{Host: s.config.KnotMirror.Url} - branchResp, err := tangled.GitTempGetBranch(r.Context(), xrpcc, pull.PullSource.Branch, sourceRepo.String()) + branchResp, err := tangled.GitTempGetBranch(r.Context(), xrpcc, pull.PullSource.Branch, sourceRepoDid) if err != nil { if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { s.logger.Error("failed to call XRPC repo.branches", "xrpcerr", xrpcerr, "err", err, "pull_id", pull.PullId, "branch", pull.PullSource.Branch) diff --git a/appview/repoverify/verify.go b/appview/repoverify/verify.go new file mode 100644 --- /dev/null +++ b/appview/repoverify/verify.go @@ -0,0 +1,155 @@ +package repoverify + +import ( + "context" + "fmt" + "net" + "net/http" + "net/url" + "syscall" + "time" + + "github.com/bluesky-social/indigo/atproto/syntax" + indigoxrpc "github.com/bluesky-social/indigo/xrpc" + "tangled.org/core/api/tangled" + "tangled.org/core/appview/xrpcclient" + "tangled.org/core/idresolver" +) + +type RepoDid syntax.DID + +func (r RepoDid) String() string { return string(r) } + +func NewRepoDid(s string) (RepoDid, error) { + did, err := syntax.ParseDID(s) + if err != nil { + return "", fmt.Errorf("invalid repoDid %q: %w", s, err) + } + return RepoDid(did), nil +} + +type OwnerDid syntax.DID + +func (o OwnerDid) String() string { return string(o) } + +func NewOwnerDid(s string) (OwnerDid, error) { + did, err := syntax.ParseDID(s) + if err != nil { + return "", fmt.Errorf("invalid ownerDid %q: %w", s, err) + } + return OwnerDid(did), nil +} + +func ParseKnotEndpoint(raw string, dev bool) (*url.URL, error) { + if raw == "" { + return nil, fmt.Errorf("empty knot URL") + } + u, err := url.Parse(raw) + if err != nil { + return nil, fmt.Errorf("invalid knot URL %q: %w", raw, err) + } + if u.Host == "" { + return nil, fmt.Errorf("knot URL %q has no host", raw) + } + switch u.Scheme { + case "https": + case "http": + if !dev { + return nil, fmt.Errorf("knot URL %q must use https outside dev mode", raw) + } + default: + return nil, fmt.Errorf("knot URL %q has unsupported scheme %q", raw, u.Scheme) + } + return u, nil +} + +type Result struct { + RepoDid RepoDid + OwnerDid OwnerDid + KnotURL *url.URL +} + +type Verifier func(ctx context.Context, repoDid RepoDid) (Result, error) + +const verifyTimeout = 10 * time.Second + +func New(resolver *idresolver.Resolver, dev bool) Verifier { + transport := &http.Transport{ + DialContext: safeDialer(dev).DialContext, + } + httpClient := &http.Client{ + Timeout: verifyTimeout, + Transport: transport, + } + + return func(ctx context.Context, repoDid RepoDid) (Result, error) { + ctx, cancel := context.WithTimeout(ctx, verifyTimeout) + defer cancel() + return resolveAndDescribe(ctx, resolver, httpClient, repoDid, dev) + } +} + +func resolveAndDescribe( + ctx context.Context, + resolver *idresolver.Resolver, + httpClient *http.Client, + repoDid RepoDid, + dev bool, +) (Result, error) { + ident, err := resolver.ResolveIdent(ctx, repoDid.String()) + if err != nil { + return Result{}, fmt.Errorf("resolve repoDid %s: %w", repoDid, err) + } + + knot, err := ParseKnotEndpoint(ident.GetServiceEndpoint("atproto_pds"), dev) + if err != nil { + return Result{}, fmt.Errorf("repoDid %s: %w", repoDid, err) + } + + client := &indigoxrpc.Client{Host: knot.String(), Client: httpClient} + out, err := tangled.RepoDescribeRepo(ctx, client, repoDid.String()) + if xrpcErr := xrpcclient.HandleXrpcErr(err); xrpcErr != nil { + return Result{}, fmt.Errorf("describeRepo on %s: %w", knot, xrpcErr) + } + + if out.RepoDid != repoDid.String() { + return Result{}, fmt.Errorf("knot %s returned mismatched repoDid: got %q, want %q", knot, out.RepoDid, repoDid) + } + + ownerDid, err := NewOwnerDid(out.OwnerDid) + if err != nil { + return Result{}, fmt.Errorf("describeRepo on %s returned invalid ownerDid: %w", knot, err) + } + + return Result{ + RepoDid: repoDid, + OwnerDid: ownerDid, + KnotURL: knot, + }, nil +} + +func safeDialer(dev bool) *net.Dialer { + d := &net.Dialer{ + Timeout: 5 * time.Second, + KeepAlive: 30 * time.Second, + } + if dev { + return d + } + d.Control = func(network, address string, _ syscall.RawConn) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + return fmt.Errorf("invalid dial address %q: %w", address, err) + } + ip := net.ParseIP(host) + if ip == nil { + return fmt.Errorf("dial address %q did not resolve to IP", address) + } + if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || + ip.IsLinkLocalMulticast() || ip.IsMulticast() || ip.IsUnspecified() { + return fmt.Errorf("refusing to dial %s: reserved or private address", ip) + } + return nil + } + return d +} diff --git a/appview/repoverify/verify_test.go b/appview/repoverify/verify_test.go new file mode 100644 --- /dev/null +++ b/appview/repoverify/verify_test.go @@ -0,0 +1,66 @@ +package repoverify + +import "testing" + +func TestNewRepoDid_RejectsInvalid(t *testing.T) { + if _, err := NewRepoDid(""); err == nil { + t.Error("NewRepoDid(\"\") err = nil, want error") + } +} + +func TestNewRepoDid_AcceptsValid(t *testing.T) { + raw := "did:plc:abc123abc123abc123abc123" + got, err := NewRepoDid(raw) + if err != nil { + t.Fatalf("NewRepoDid(%q): %v", raw, err) + } + if got.String() != raw { + t.Errorf("got %q, want %q", got, raw) + } +} + +func TestParseKnotEndpoint_RejectsHttpInProd(t *testing.T) { + if _, err := ParseKnotEndpoint("http://knot.example", false); err == nil { + t.Error("http:// knot URL accepted in prod") + } +} + +func TestParseKnotEndpoint_AllowsHttpInDev(t *testing.T) { + u, err := ParseKnotEndpoint("http://knot.example", true) + if err != nil { + t.Fatalf("dev mode should allow http: %v", err) + } + if u.Host != "knot.example" { + t.Errorf("Host = %q, want knot.example", u.Host) + } +} + +func TestParseKnotEndpoint_RejectsUnsupportedScheme(t *testing.T) { + if _, err := ParseKnotEndpoint("ftp://knot.example", true); err == nil { + t.Error("ParseKnotEndpoint accepted ftp:// in dev") + } + if _, err := ParseKnotEndpoint("ftp://knot.example", false); err == nil { + t.Error("ParseKnotEndpoint accepted ftp:// in prod") + } +} + +func TestParseKnotEndpoint_RejectsEmptyOrHostless(t *testing.T) { + cases := []string{"", "https://", "not a url at all"} + for _, raw := range cases { + t.Run(raw, func(t *testing.T) { + if _, err := ParseKnotEndpoint(raw, false); err == nil { + t.Errorf("ParseKnotEndpoint(%q) accepted bogus URL", raw) + } + }) + } +} + +func TestParseKnotEndpoint_HostPreservesPort(t *testing.T) { + u, err := ParseKnotEndpoint("http://localhost:3000", true) + if err != nil { + t.Fatalf("ParseKnotEndpoint: %v", err) + } + if u.Host != "localhost:3000" { + t.Errorf("Host = %q, want localhost:3000", u.Host) + } +} diff --git a/appview/state/state.go b/appview/state/state.go --- a/appview/state/state.go +++ b/appview/state/state.go @@ -29,6 +29,7 @@ "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages" "tangled.org/core/appview/reporesolver" + "tangled.org/core/appview/repoverify" "tangled.org/core/appview/validator" xrpcclient "tangled.org/core/appview/xrpcclient" "tangled.org/core/consts" @@ -181,6 +182,7 @@ Logger: log.SubLogger(logger, "ingester"), Validator: validator, Notifier: notifier, + Verifier: repoverify.New(res, config.Core.Dev), } err = jc.StartJetstream(ctx, ingester.Ingest()) if err != nil { diff --git a/knotserver/xrpc/repo_describe_repo.go b/knotserver/xrpc/repo_describe_repo.go new file mode 100644 --- /dev/null +++ b/knotserver/xrpc/repo_describe_repo.go @@ -0,0 +1,39 @@ +package xrpc + +import ( + "database/sql" + "errors" + "net/http" + + "github.com/bluesky-social/indigo/atproto/syntax" + "tangled.org/core/api/tangled" + xrpcerr "tangled.org/core/xrpc/errors" +) + +func (x *Xrpc) RepoDescribeRepo(w http.ResponseWriter, r *http.Request) { + raw := r.URL.Query().Get("repoDid") + repoDid, err := syntax.ParseDID(raw) + if err != nil { + writeError(w, xrpcerr.NewXrpcError( + xrpcerr.WithTag("InvalidRequest"), + xrpcerr.WithMessage("missing or invalid repoDid parameter"), + ), http.StatusBadRequest) + return + } + + ownerDid, rkey, err := x.Db.GetRepoKeyOwner(repoDid.String()) + if errors.Is(err, sql.ErrNoRows) { + writeError(w, xrpcerr.RepoNotFoundError, http.StatusNotFound) + return + } + if err != nil { + writeError(w, xrpcerr.GenericError(err), http.StatusInternalServerError) + return + } + + x.writeJson(w, tangled.RepoDescribeRepo_Output{ + RepoDid: repoDid.String(), + OwnerDid: ownerDid, + Rkey: rkey, + }) +} diff --git a/knotserver/xrpc/repo_describe_repo_test.go b/knotserver/xrpc/repo_describe_repo_test.go new file mode 100644 --- /dev/null +++ b/knotserver/xrpc/repo_describe_repo_test.go @@ -0,0 +1,94 @@ +package xrpc + +import ( + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "path/filepath" + "testing" + + "tangled.org/core/api/tangled" + "tangled.org/core/knotserver/config" + "tangled.org/core/knotserver/db" +) + +func newTestXrpc(t *testing.T) *Xrpc { + t.Helper() + d, err := db.Setup(context.Background(), filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatalf("db.Setup: %v", err) + } + return &Xrpc{ + Db: d, + Config: &config.Config{Server: config.Server{Hostname: "knot.example", MaxResponseKB: 5120}}, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + } +} + +func TestRepoDescribeRepo_ReturnsOwner(t *testing.T) { + x := newTestXrpc(t) + if err := x.Db.StoreRepoKey("did:plc:repo1", []byte("dummy"), "did:plc:akshay", "myrepo"); err != nil { + t.Fatalf("StoreRepoKey: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/xrpc/sh.tangled.repo.describeRepo?repoDid=did:plc:repo1", nil) + rec := httptest.NewRecorder() + x.RepoDescribeRepo(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + + var out tangled.RepoDescribeRepo_Output + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode: %v", err) + } + if out.RepoDid != "did:plc:repo1" { + t.Errorf("RepoDid = %q", out.RepoDid) + } + if out.OwnerDid != "did:plc:akshay" { + t.Errorf("OwnerDid = %q, want did:plc:akshay", out.OwnerDid) + } + if out.Rkey != "myrepo" { + t.Errorf("Rkey = %q, want myrepo", out.Rkey) + } +} + +func TestRepoDescribeRepo_UnknownRepoDidReturns404(t *testing.T) { + x := newTestXrpc(t) + + req := httptest.NewRequest(http.MethodGet, "/xrpc/sh.tangled.repo.describeRepo?repoDid=did:plc:unknown", nil) + rec := httptest.NewRecorder() + x.RepoDescribeRepo(rec, req) + + if rec.Code != http.StatusNotFound { + t.Errorf("status = %d, want 404", rec.Code) + } +} + +func TestRepoDescribeRepo_MissingParamReturns400(t *testing.T) { + x := newTestXrpc(t) + + req := httptest.NewRequest(http.MethodGet, "/xrpc/sh.tangled.repo.describeRepo", nil) + rec := httptest.NewRecorder() + x.RepoDescribeRepo(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", rec.Code) + } +} + +func TestRepoDescribeRepo_MalformedDidParamReturns400(t *testing.T) { + x := newTestXrpc(t) + u := "/xrpc/sh.tangled.repo.describeRepo?repoDid=" + url.QueryEscape("notadid") + req := httptest.NewRequest(http.MethodGet, u, nil) + rec := httptest.NewRecorder() + x.RepoDescribeRepo(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", rec.Code) + } +} diff --git a/knotserver/xrpc/xrpc.go b/knotserver/xrpc/xrpc.go --- a/knotserver/xrpc/xrpc.go +++ b/knotserver/xrpc/xrpc.go @@ -67,6 +67,7 @@ r.Get("/"+tangled.RepoDiffNSID, x.RepoDiff) r.Get("/"+tangled.RepoCompareNSID, x.RepoCompare) r.Get("/"+tangled.RepoGetDefaultBranchNSID, x.RepoGetDefaultBranch) + r.Get("/"+tangled.RepoDescribeRepoNSID, x.RepoDescribeRepo) r.Get("/"+tangled.RepoBranchNSID, x.RepoBranch) r.Get("/"+tangled.RepoArchiveNSID, x.RepoArchive) r.Get("/"+tangled.RepoLanguagesNSID, x.RepoLanguages) diff --git a/lexicons/repo/describeRepo.json b/lexicons/repo/describeRepo.json new file mode 100644 --- /dev/null +++ b/lexicons/repo/describeRepo.json @@ -0,0 +1,53 @@ +{ + "lexicon": 1, + "id": "sh.tangled.repo.describeRepo", + "defs": { + "main": { + "type": "query", + "description": "Fetch the knot's authoritative metadata for a git repo DID.", + "parameters": { + "type": "params", + "required": ["repoDid"], + "properties": { + "repoDid": { + "type": "string", + "format": "did", + "description": "DID of the git repo as minted by the knot" + } + } + }, + "output": { + "encoding": "application/json", + "schema": { + "type": "object", + "required": ["repoDid", "ownerDid", "rkey"], + "properties": { + "repoDid": { + "type": "string", + "format": "did" + }, + "ownerDid": { + "type": "string", + "format": "did", + "description": "DID of the current owner according to the knot." + }, + "rkey": { + "type": "string", + "description": "Current rkey of the sh.tangled.repo record tracked by this knot" + } + } + } + }, + "errors": [ + { + "name": "RepoNotFound", + "description": "Repo DID is not registered on this knot" + }, + { + "name": "InvalidRequest", + "description": "Invalid request parameters" + } + ] + } + } +}