diff --git a/internal/ingest/forged_attribution_test.go b/internal/ingest/forged_attribution_test.go new file mode 100644 index 0000000..a889047 --- /dev/null +++ b/internal/ingest/forged_attribution_test.go @@ -0,0 +1,58 @@ +package ingest + +import ( + "context" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "tidepool/internal/ap" + "tidepool/internal/errors" +) + +// TestBareCreateCannotAttributeToAnotherInstance: bare delivery is signed by +// whoever sent it, and nothing about that signature vouches for the content's +// `attributedTo`. Since the postv2 flip a materialized post lands in the +// AUTHOR's repo and is signed by that repo's key, so an accepted forgery here +// is a signed post the victim never wrote. The delivering instance may only +// ever attribute content to its own users — which is exactly what Lemmy's +// verify_domains_match already guarantees of genuine traffic. +func TestBareCreateCannotAttributeToAnotherInstance(t *testing.T) { + h := newHarness(t) + h.subscribeTechnology() + // The victim's actor document is fetchable, so the refusal below is the + // attribution check and not a failed mint. + h.serveLemmyWorldContent() + ctx := context.Background() + + const forgedID = "https://evil.example/post/1" + eve := h.newRemoteActor("https://evil.example/u/eve", person("https://evil.example/u/eve", "eve", nil)) + + require.Equal(t, http.StatusAccepted, h.deliver(eve, map[string]any{ + "id": "https://evil.example/activities/create/1", + "type": "Create", + "actor": eve.id, + "object": map[string]any{ + "type": "Page", + "id": forgedID, + "attributedTo": personID, // a user on lemmy.world, not on evil.example + "to": []any{ap.PublicAudience}, + "audience": groupID, + "name": "a post the victim never wrote", + "source": map[string]any{"content": "body", "mediaType": "text/markdown"}, + "published": "2026-07-08T17:00:00.000000Z", + }, + })) + h.drain() + + event, err := h.events.GetEvent(ctx, "https://evil.example/activities/create/1") + require.NoError(t, err) + assert.NotNil(t, event.ProcessedAt, "the drop is a processed skip, never a retry") + + _, err = h.objects.GetByAPID(ctx, forgedID) + assert.True(t, errors.IsNotFound(err), "forged content must not be materialized") + _, err = h.actors.GetByAPActorID(ctx, personID) + assert.True(t, errors.IsNotFound(err), "naming a victim must not bridge them") +} diff --git a/internal/materialize/comments.go b/internal/materialize/comments.go index 89f0e6d..1ac3c35 100644 --- a/internal/materialize/comments.go +++ b/internal/materialize/comments.go @@ -178,11 +178,38 @@ func (m *Materializer) materializeCommentLeaf(ctx context.Context, note *ap.Obje if authorRef == nil || authorRef.ID == "" { return nil, skip(note.ID, "comment has no attributedTo author") } + // Before anything is minted: a forged attribution must not cost the actor + // it names a DID. + if err := requireSameAuthorityAuthor(note, authorRef); err != nil { + return nil, err + } author, err := m.EnsureActor(ctx, authorRef) if err != nil { return nil, err } + did, authorDID := author.DID, author.DID + if existing, err := m.objects.GetByAPID(ctx, note.ID); err == nil { + // The repo a comment lives in IS its authorship claim, so authorship — + // and with it the record's coordinates — is fixed at first + // materialization, exactly as MaterializePost pins a post's. attributedTo + // on an updated Note is proposed by whoever delivered the update: + // honouring a changed value would sign the record with an unrelated + // bridged user's repo key and strand the real author's copy live at its + // old at-uri. rkey is pinned with it because it is derived from + // `published`, which an edit can also restate. + // + // The collection is NOT pinned: unlike posts, comments never moved + // between collections, so CollectionComment is the only answer in either + // era and re-deriving it cannot relocate anything. + did, rkey = existing.DID, existing.RKey + if existing.AuthorDID != "" { + authorDID = existing.AuthorDID + } + } else if !errors.IsNotFound(err) { + return nil, fmt.Errorf("materialize: check mapping for %s: %w", note.ID, err) + } + reply, communityDID, err := m.resolveReplyRefs(ctx, note) if err != nil { return nil, err @@ -214,7 +241,7 @@ func (m *Materializer) materializeCommentLeaf(ctx context.Context, note *ap.Obje if note.Sensitive != nil && *note.Sensitive { record["labels"] = selfLabels("nsfw") } - return m.commitRecord(ctx, author.DID, CollectionComment, rkey, record, note, author.DID, communityDID) + return m.commitRecord(ctx, did, CollectionComment, rkey, record, note, authorDID, communityDID) } // resolveReplyRefs builds the reply {root, parent} strongRefs for a diff --git a/internal/materialize/forged_attribution_test.go b/internal/materialize/forged_attribution_test.go new file mode 100644 index 0000000..0e227bd --- /dev/null +++ b/internal/materialize/forged_attribution_test.go @@ -0,0 +1,130 @@ +package materialize + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "tidepool/internal/errors" +) + +// Forged attribution. Since the postv2 flip the repo a record lands in IS the +// authorship claim — the record is signed by that repo's key — so `attributedTo` +// on delivered content decides whose signature ends up on it. These tests pin +// the two rules that keeps honest: an already-materialized object never moves +// repos, and an object may only ever attribute itself to an actor on its own +// authority. + +// TestCommentEditCannotReattributeAuthor: an Update{Note} that names a +// different author must not relocate the comment. The mapping is the authority +// on who wrote a bridged object — exactly as MaterializePost already treats it +// — because honouring the edit would sign the record with an unrelated user's +// repo key and strand the original copy live in the first author's repo. +func TestCommentEditCannotReattributeAuthor(t *testing.T) { + h := newHarness(t) + h.serveLemmyWorldFixtures() + h.serveObject("/u/alice", person("https://lemmy.world/u/alice", "alice", nil)) + h.serveObject("/u/victim", person("https://lemmy.world/u/victim", "victim", nil)) + ctx := context.Background() + + _, err := h.m.MaterializePost(ctx, loadFixtureObject(t, "page_lemmy_world.json")) + require.NoError(t, err) + + const commentID = "https://lemmy.world/comment/80001" + original := note(commentID, "https://lemmy.world/u/alice", pageID, + "alice wrote this", "2026-07-08T16:00:00.000000Z") + _, err = h.m.MaterializeComment(ctx, objectFromMap(t, original)) + require.NoError(t, err) + + aliceDID := testDIDFor("alice", "lemmy.world") + victimDID := testDIDFor("victim", "lemmy.world") + require.NotEqual(t, aliceDID, victimDID) + + before, err := h.objects.GetByAPID(ctx, commentID) + require.NoError(t, err) + require.Equal(t, aliceDID, before.DID, "precondition: the comment lives in alice's repo") + require.Equal(t, aliceDID, before.AuthorDID) + + // The edit re-attributes the comment to another user on the SAME instance, + // so the authority check below cannot be what refuses it. + forged := note(commentID, "https://lemmy.world/u/victim", pageID, + "alice wrote this (edited)", "2026-07-08T16:00:00.000000Z") + _, err = h.m.HandleUpdate(ctx, objectFromMap(t, forged)) + require.NoError(t, err, "a re-attributed edit must not error — it must simply not re-attribute") + + after, err := h.objects.GetByAPID(ctx, commentID) + require.NoError(t, err) + assert.Equal(t, aliceDID, after.DID, + "the comment must stay in the repo that authored it: the repo IS the authorship claim, so "+ + "moving it signs the victim's key over content they never wrote") + assert.Equal(t, aliceDID, after.AuthorDID, "the stored author is fixed at first materialization") + assert.Equal(t, before.ATURI, after.ATURI, + "re-pointing the mapping would strand the original record live in alice's repo") + + _, _, err = h.manager.GetRecord(ctx, aliceDID, CollectionComment, after.RKey) + require.NoError(t, err, "the comment must still be readable where it was written") + + entries, err := h.manager.ListRecords(ctx, victimDID) + require.NoError(t, err) + for _, entry := range entries { + assert.NotEqual(t, CollectionComment, entry.Collection, + "no comment may be written into the named victim's repo (rkey %s)", entry.Rkey) + } +} + +// TestCommentCrossAuthorityAttributionRefused: a Note served by one instance +// may not attribute itself to a user on another. Lemmy enforces the same rule +// on its own inbound path (verify_domains_match), so genuine traffic never +// trips this — but without it any instance can name any bridged user as the +// author of anything it delivers. +func TestCommentCrossAuthorityAttributionRefused(t *testing.T) { + h := newHarness(t) + h.serveLemmyWorldFixtures() + ctx := context.Background() + + // The thread root is real and materialized, so a refusal below cannot be + // the missing-parent protocol talking. + _, err := h.m.MaterializePost(ctx, loadFixtureObject(t, "page_lemmy_world.json")) + require.NoError(t, err) + + const victimIRI = "https://lemmy.world/u/victim" + const forgedID = "https://evil.example/comment/1" + h.serveObject("/u/victim", person(victimIRI, "victim", nil)) + + res, err := h.m.MaterializeComment(ctx, objectFromMap(t, + note(forgedID, victimIRI, pageID, "words the victim never wrote", "2026-07-08T16:10:00.000000Z"))) + require.Nil(t, res) + require.Error(t, err) + assert.True(t, IsSkip(err), "cross-authority attribution must be a skip, got %v", err) + + assert.Equal(t, 0, countMappings(t, h, forgedID)) + _, err = h.actors.GetByAPActorID(ctx, victimIRI) + assert.True(t, errors.IsNotFound(err), + "the refusal must precede the mint: naming a victim must not even bridge them") +} + +// TestPostCrossAuthorityAttributionRefused is the same rule on the post path. +// A postv2 carries no `author` field at all — its repo is the whole claim — so +// an accepted forgery here is indistinguishable from a post the victim wrote. +func TestPostCrossAuthorityAttributionRefused(t *testing.T) { + h := newHarness(t) + h.serveLemmyWorldFixtures() + ctx := context.Background() + + const forgedID = "https://evil.example/post/1" + res, err := h.m.MaterializePost(ctx, mustObject(t, + page(forgedID, personID, groupID, "not their post", "2026-07-08T16:20:00.000000Z"))) + require.Nil(t, res) + require.Error(t, err) + assert.True(t, IsSkip(err), "cross-authority attribution must be a skip, got %v", err) + + assert.Equal(t, 0, countMappings(t, h, forgedID)) + _, err = h.actors.GetByAPActorID(ctx, personID) + assert.True(t, errors.IsNotFound(err), + "the refusal must precede the mint: naming a victim must not even bridge them") + _, err = h.communities.GetByAPGroupID(ctx, groupID) + assert.True(t, errors.IsNotFound(err), + "forged content must not bridge the community it claims either") +} diff --git a/internal/materialize/materializer.go b/internal/materialize/materializer.go index c2ab75f..33f14e9 100644 --- a/internal/materialize/materializer.go +++ b/internal/materialize/materializer.go @@ -116,6 +116,28 @@ func IsSkip(err error) bool { return stderrors.Is(err, ErrSkipped) } func skip(apID, reason string) error { return &SkipError{APID: apID, Reason: reason} } +// requireSameAuthorityAuthor refuses content that attributes itself to an actor +// on a DIFFERENT authority than the object's own id. +// +// Since the postv2 flip, the repo a record lands in IS its authorship claim: +// the commit is signed by that repo's key, and a postv2 carries no `author` +// field for a consumer to disagree with. attributedTo is therefore the field +// that decides whose signature ends up on delivered content, and it is written +// by whoever served the object. Without this check any instance can hand the +// bridge a Page or Note naming any bridged user and have it signed into that +// user's repo. +// +// Lemmy binds the two itself on ITS inbound path (verify_domains_match over an +// object's id and its creator), so genuine Lemmy traffic — including a +// lemmy.zip user's post announced by a lemmy.world community — never fails +// this; only the id's own host may speak for its users. +func requireSameAuthorityAuthor(obj, authorRef *ap.Object) error { + if !ap.SameAuthority(obj.ID, authorRef.ID) { + return skip(obj.ID, "attributedTo "+authorRef.ID+" is on another authority") + } + return nil +} + // Fetcher is the slice of the AP client the materializer uses. *ap.Client // implements it; tests may substitute failures. type Fetcher interface { diff --git a/internal/materialize/posts.go b/internal/materialize/posts.go index daa0506..2b095e5 100644 --- a/internal/materialize/posts.go +++ b/internal/materialize/posts.go @@ -36,6 +36,16 @@ func (m *Materializer) MaterializePost(ctx context.Context, page *ap.Object) (*R return nil, err } + authorRef := page.AttributedTo.First() + if authorRef == nil || authorRef.ID == "" { + return nil, skip(page.ID, "post has no attributedTo author") + } + // Before anything is minted: a forged attribution must not cost the actor + // it names a DID, nor the community it claims a repo. + if err := requireSameAuthorityAuthor(page, authorRef); err != nil { + return nil, err + } + groupRef := communityRef(page) if groupRef == nil { return nil, skip(page.ID, "post names no community (no audience/to group IRI)") @@ -44,10 +54,6 @@ func (m *Materializer) MaterializePost(ctx context.Context, page *ap.Object) (*R if err != nil { return nil, err } - authorRef := page.AttributedTo.First() - if authorRef == nil || authorRef.ID == "" { - return nil, skip(page.ID, "post has no attributedTo author") - } author, err := m.EnsureActor(ctx, authorRef) if err != nil { return nil, err diff --git a/internal/materialize/security_test.go b/internal/materialize/security_test.go index ee8f15a..ff6ab01 100644 --- a/internal/materialize/security_test.go +++ b/internal/materialize/security_test.go @@ -64,18 +64,20 @@ func TestCommentThreadRootedAtNote_SkipsWithoutPanic(t *testing.T) { h := newHarness(t) ctx := context.Background() - // A root Note with no inReplyTo, served upstream. + // A root Note with no inReplyTo, served upstream. Author and object share + // an authority throughout, as genuine traffic does, so the skip below can + // only be the parentless root and not the attribution check. rootNote := map[string]any{ "type": "Note", "id": "https://lemmy.zip/comment/root", - "attributedTo": personID, + "attributedTo": "https://lemmy.zip/u/carol", "audience": groupID, "source": map[string]any{"content": "root", "mediaType": "text/markdown"}, "published": "2024-01-02T00:00:00.000000Z", } h.serveObject("/comment/root", rootNote) - child := note("https://lemmy.zip/comment/child", personID, + child := note("https://lemmy.zip/comment/child", "https://lemmy.zip/u/carol", "https://lemmy.zip/comment/root", "child", "2024-01-02T01:00:00.000000Z") res, err := h.m.MaterializeComment(ctx, mustObject(t, child)) diff --git a/internal/store/ap_objects.go b/internal/store/ap_objects.go index 6fac602..4daa530 100644 --- a/internal/store/ap_objects.go +++ b/internal/store/ap_objects.go @@ -64,18 +64,23 @@ func (r *postgresAPObjects) putMapping(ctx context.Context, q queryRower, mappin ap_type = EXCLUDED.ap_type, origin = EXCLUDED.origin, did = EXCLUDED.did, - -- author_did gets the same COALESCE treatment, and for a sharper - -- reason than tidiness: deleteIsByAuthor decides SELF-DELETE vs - -- MODERATOR REMOVAL from this column, and a re-put that omitted it - -- would silently turn every later author delete into "not provably - -- the author" — the branch that writes a moderation record. - author_did = COALESCE(EXCLUDED.author_did, ap_objects.author_did), - -- COALESCE, never a bare overwrite: community_did is the binding - -- that authorizes announced moderation of this object, and a - -- re-put that simply omits it (a re-materialization, a legacy - -- write path) would NULL a good binding and make moderation refuse - -- forever, silently. A write that HAS the value still wins. - community_did = COALESCE(EXCLUDED.community_did, ap_objects.community_did), + -- The STORED value first: author_did is immutable once known. Since + -- the postv2 flip it records whose repo signed the record — the + -- strongest authorship statement atproto has — and it is also how + -- deleteIsByAuthor decides SELF-DELETE vs MODERATOR REMOVAL. A re-put + -- that omitted it would silently turn every later author delete into + -- "not provably the author", and one that restated it differently + -- would re-attribute bridged content to somebody who never wrote it. + -- A NULL is still filled by the first write that knows the answer. + author_did = COALESCE(ap_objects.author_did, EXCLUDED.author_did), + -- Same rule, same reason: community_did is the binding that + -- authorizes announced moderation of this object, so an object's + -- community is decided once. A re-put that omits it (a + -- re-materialization, a legacy write path) must not NULL a good + -- binding and make moderation refuse forever, and a re-put that names + -- a DIFFERENT community must not hand that community moderation + -- authority over content posted somewhere else. + community_did = COALESCE(ap_objects.community_did, EXCLUDED.community_did), -- COALESCE for the same reason, with one difference worth stating: -- the materializer re-derives this from the record it is committing, -- so a re-put normally re-supplies it and a row written before diff --git a/internal/store/ap_objects_test.go b/internal/store/ap_objects_test.go index 481887a..c0d04f2 100644 --- a/internal/store/ap_objects_test.go +++ b/internal/store/ap_objects_test.go @@ -191,6 +191,56 @@ func TestAPObjects_PutMapping_Validation(t *testing.T) { } } +// TestAPObjects_PutMapping_AuthorAndCommunityAreImmutable: once a mapping +// names an author and a community, a later upsert may not move either. Both +// columns are authorization inputs — author_did is how deleteIsByAuthor tells a +// self-delete from a moderator removal, community_did is what authorizes +// announced moderation — and since the postv2 flip author_did also records +// whose repo signed the record. Making that structural in SQL is what stops a +// single careless (or forged) write path from re-attributing bridged content; +// per-caller discipline only holds until the next caller. +func TestAPObjects_PutMapping_AuthorAndCommunityAreImmutable(t *testing.T) { + repo := NewAPObjects(testDB(t)) + ctx := context.Background() + + first := testMapping() + first.AuthorDID = testDID + first.CommunityDID = testDID + stored, err := repo.PutMapping(ctx, first) + require.NoError(t, err) + require.Equal(t, testDID, stored.AuthorDID) + require.Equal(t, testDID, stored.CommunityDID) + + reattributed := testMapping() + reattributed.AuthorDID = testSecondDID + reattributed.CommunityDID = testSecondDID + stored, err = repo.PutMapping(ctx, reattributed) + require.NoError(t, err) + assert.Equal(t, testDID, stored.AuthorDID, "a re-put must not re-attribute an object to another author") + assert.Equal(t, testDID, stored.CommunityDID, "a re-put must not move an object into another community") + + // An omitted value still cannot blank a stored one, and a first value + // still lands: immutability is "the first non-empty write wins", not + // "writes after the insert are ignored". + blanked := testMapping() + stored, err = repo.PutMapping(ctx, blanked) + require.NoError(t, err) + assert.Equal(t, testDID, stored.AuthorDID) + assert.Equal(t, testDID, stored.CommunityDID) + + late := testMapping() + late.APID = "https://lemmy.world/post/54321" + late.RKey = "3jzfcijpj2z2b" + _, err = repo.PutMapping(ctx, late) + require.NoError(t, err) + late.AuthorDID = testSecondDID + late.CommunityDID = testSecondDID + stored, err = repo.PutMapping(ctx, late) + require.NoError(t, err) + assert.Equal(t, testSecondDID, stored.AuthorDID, "a NULL author is still filled by the first write that knows it") + assert.Equal(t, testSecondDID, stored.CommunityDID, "a NULL community is still filled by the first write that knows it") +} + func TestAPObjects_PutMapping_ATURICollisionIsConflict(t *testing.T) { repo := NewAPObjects(testDB(t)) ctx := context.Background()