diff --git a/cmd/tidepool/main.go b/cmd/tidepool/main.go index 37e6521..be5a424 100644 --- a/cmd/tidepool/main.go +++ b/cmd/tidepool/main.go @@ -342,8 +342,10 @@ func run(logger *slog.Logger) error { Materializer: materializer, Fetcher: apClient, Objects: objects, + Actors: actors, Communities: communities, Tombstones: tombstones, + Records: repoManager, Votes: voteAggregator, Backfill: backfill, ServiceActorID: serviceActor.ID, diff --git a/internal/db/migrations/015_scoped_tombstones.sql b/internal/db/migrations/015_scoped_tombstones.sql new file mode 100644 index 0000000..d023304 --- /dev/null +++ b/internal/db/migrations/015_scoped_tombstones.sql @@ -0,0 +1,47 @@ +-- +goose Up +-- ap_tombstones markers were keyed by ap_id alone, so every marker suppressed +-- its id GLOBALLY. Combined with authorizeDelete's deliberate allowance for an +-- UNMAPPED announced target (needed so the delete-before-create race can still +-- leave a marker), that made the table a cross-community suppression +-- primitive: any ONE followed community could announce Delete{} and +-- pre-suppress an id belonging to a DIFFERENT community for the whole +-- TOMBSTONE_RETENTION window (30d default), because materializeContent and the +-- backfill honoured the marker no matter who laid it. +-- +-- announcer scopes the marker to the authority that laid it: the announcing +-- community's AP group id, or '' for an ORIGIN-authorized marker (a bare +-- same-authority Delete, or the admin sweep's origin-verified 410) which stays +-- global. Reads ask for markers visible in their own context — global, or laid +-- by the community whose announce is being processed — so community A's marker +-- can no longer suppress community B's content. The primary key moves to +-- (ap_id, announcer) so two communities' independent markers for the same id +-- coexist instead of the first one winning the ON CONFLICT. +-- +-- Existing rows become announcer = '' (global). That is the conservative +-- reading of history: the old markers were laid by a mix of origin-authorized +-- and community-announced deletes that we can no longer tell apart, and +-- keeping them global preserves the suppression they already provide (a marker +-- that stops resurrecting deleted content is the safe direction to err in; +-- retention prunes them within 30 days anyway). It is also sound rather than +-- merely conservative: every row that exists in PRODUCTION was recorded under +-- the ORIGINAL same-authority-only delete rule, i.e. by the target id's own +-- host — the intermediate revision that let a followed community announce a +-- delete for an unmapped foreign id was never deployed. Promoting those rows +-- to global states what they already were. +ALTER TABLE ap_tombstones ADD COLUMN announcer TEXT NOT NULL DEFAULT ''; + +ALTER TABLE ap_tombstones DROP CONSTRAINT ap_tombstones_pkey; +ALTER TABLE ap_tombstones ADD PRIMARY KEY (ap_id, announcer); + +-- +goose Down +-- Collapse back to one row per ap_id, keeping the OLDEST marker (its +-- deleted_at is what retention pruning reads, so keeping the oldest preserves +-- the original prune schedule rather than extending it). +DELETE FROM ap_tombstones a + USING ap_tombstones b + WHERE a.ap_id = b.ap_id + AND (b.deleted_at, b.announcer) < (a.deleted_at, a.announcer); + +ALTER TABLE ap_tombstones DROP CONSTRAINT ap_tombstones_pkey; +ALTER TABLE ap_tombstones ADD PRIMARY KEY (ap_id); +ALTER TABLE ap_tombstones DROP COLUMN IF EXISTS announcer; diff --git a/internal/ingest/backfill.go b/internal/ingest/backfill.go index 67793ac..4bf1b5f 100644 --- a/internal/ingest/backfill.go +++ b/internal/ingest/backfill.go @@ -268,7 +268,10 @@ func (b *Backfill) materializeOutboxItem(ctx context.Context, item *ap.Object, c // Same funnel rules as live deliveries: never resurrect deleted // content, trust embedded bodies only on the outbox host's authority. - tombstoned, err := b.tombstones.Exists(ctx, obj.ID) + // The walk reads markers in the backfilled community's scope — its own, + // plus origin-authorized ones; a marker another community laid says + // nothing about this community's outbox. + tombstoned, err := b.tombstones.ExistsFor(ctx, obj.ID, communityIRI) if err != nil { return false, fmt.Errorf("ingest: tombstone check for %s: %w", obj.ID, err) } @@ -286,7 +289,7 @@ func (b *Backfill) materializeOutboxItem(ctx context.Context, item *ap.Object, c return false, err } b.seedCounts(ctx, obj.ID) - b.backfillReplies(ctx, obj) + b.backfillReplies(ctx, obj, communityIRI) return true, nil case ap.TypeNote: if _, err := b.mat.MaterializeComment(ctx, obj); err != nil { @@ -318,7 +321,9 @@ func (b *Backfill) seedCounts(ctx context.Context, postAPID string) { // backfillReplies pages a post's advertised replies collection. Failures // are logged, never fatal — replies are best-effort garnish on backfill. -func (b *Backfill) backfillReplies(ctx context.Context, post *ap.Object) { +// communityIRI is the community being backfilled, carried for the scoped +// tombstone lookup below. +func (b *Backfill) backfillReplies(ctx context.Context, post *ap.Object, communityIRI string) { if post.Replies == nil || post.Replies.ID == "" { // Not advertised (or inline-only, which Lemmy never emits). return @@ -342,7 +347,7 @@ func (b *Backfill) backfillReplies(ctx context.Context, post *ap.Object) { // Same funnel rule as the live path and materializeOutboxItem: a reply // with a recorded Delete must never be resurrected, even if it still // lingers in the origin's replies collection (delivery/collection race). - tombstoned, err := b.tombstones.Exists(ctx, resolved.ID) + tombstoned, err := b.tombstones.ExistsFor(ctx, resolved.ID, communityIRI) if err != nil { b.logger.Warn("backfill reply tombstone check failed", "post", post.ID, "reply", resolved.ID, "error", err) return nil diff --git a/internal/ingest/backfill_test.go b/internal/ingest/backfill_test.go index e2615d2..550a302 100644 --- a/internal/ingest/backfill_test.go +++ b/internal/ingest/backfill_test.go @@ -190,7 +190,9 @@ func TestBackfillSkipsTombstonedReplies(t *testing.T) { ctx := context.Background() const replyID = "https://lemmy.world/comment/3001" - require.NoError(t, h.tombstones.Record(ctx, replyID)) + // Scoped to the community being backfilled: its own marker must hold on + // its own walk (the global-marker case is covered below). + require.NoError(t, h.tombstones.Record(ctx, replyID, groupID)) community, err := h.communities.GetByAPGroupID(ctx, groupID) require.NoError(t, err) @@ -332,7 +334,8 @@ func TestBackfillSkipsTombstonedObjects(t *testing.T) { b := newBackfill(t, h, 10) ctx := context.Background() - require.NoError(t, h.tombstones.Record(ctx, pageID)) + // An origin-authorized (global) marker: visible on every community's walk. + require.NoError(t, h.tombstones.Record(ctx, pageID, "")) community, err := h.communities.GetByAPGroupID(ctx, groupID) require.NoError(t, err) require.NoError(t, b.Run(ctx, community, true)) diff --git a/internal/ingest/consent.go b/internal/ingest/consent.go index 2121965..333807e 100644 --- a/internal/ingest/consent.go +++ b/internal/ingest/consent.go @@ -22,10 +22,12 @@ package ingest import ( "context" "fmt" + "strings" "tidepool/internal/ap" "tidepool/internal/errors" "tidepool/internal/materialize" + "tidepool/internal/store" ) // applyProfileUpdate handles Update{Person|Group}. The embedded document is @@ -69,14 +71,20 @@ func (h *Handler) applyProfileUpdate(ctx context.Context, actorDoc *ap.Object, s } // handleDelete processes Delete{object-or-actor}. announcer is the -// announcing community's AP id ("" when delivered bare). +// announcing community, already resolved by handleAnnounce (nil when the +// activity was delivered bare). // -// Authorization: a Delete announced by a followed community is trusted (the -// community moderates its own content — Lemmy only announces deletes for -// objects in its communities). A bare Delete must come from the deleted +// Authorization: a Delete announced by a followed community is trusted for +// records that belong to that community — its own repo for posts, its +// thread root for comments (the community moderates the content posted into +// it, wherever the author lives). A bare Delete must come from the deleted // id's own authority (the actor deleting their content/account, or their -// instance acting for them). Anything else is dropped. -func (h *Handler) handleDelete(ctx context.Context, del *ap.Object, signer, announcer string) error { +// instance acting for them). An announced delete of an id the bridge has no +// mapping for is accepted too, but reaches only the tombstone marker below: +// nothing is removed because nothing was ever materialized, and the marker is +// scoped to the announcing community so it cannot suppress anyone else's +// content. Everything else drops. +func (h *Handler) handleDelete(ctx context.Context, del *ap.Object, signer string, announcer *store.Community) error { targetID := refID(del.Object) if targetID == "" { return errors.NewValidationError("delete", "delete carries no object id") @@ -88,28 +96,53 @@ func (h *Handler) handleDelete(ctx context.Context, del *ap.Object, signer, anno // Record the tombstone marker BEFORE deleting: if this is a Delete for // an object we never materialized, the marker is the only thing // stopping a later (re-delivered, out-of-order) Create from - // resurrecting it. - if err := h.tombstones.Record(ctx, targetID); err != nil { + // resurrecting it. Scoped to the announcing community; a bare delivery + // passed the same-authority origin check, so its marker is global. + if err := h.tombstones.Record(ctx, targetID, announcerGroupID(announcer)); err != nil { return fmt.Errorf("ingest: record tombstone for %s: %w", targetID, err) } - // HandleDelete branches actor vs object itself: a known actor id runs - // the full Delete(Actor) scrub (records tombstoned, consent → deleted, - // terminal); an object id deletes the record and soft-deletes its - // mapping; an unknown id is a logged no-op. - if err := h.mat.HandleDelete(ctx, targetID); err != nil { + // Actor vs content is decided ONCE, by authorizeDelete, and the dispatch + // here carries that decision into the materializer instead of letting it + // re-derive one. An ANNOUNCED delete can only ever have been authorized + // for CONTENT (every actor target — bridged_actors row or profile mapping + // — is refused above), so it takes the content-only entry: HandleDelete + // would re-read bridged_actors, and an actor being minted right now has + // neither row when authorizeDelete looks and an actor row by the time the + // materializer looks, which would turn an unrelated community's announced + // delete into that actor's terminal scrub. A BARE delete keeps the full + // dispatch — Delete(Actor) arrives on exactly that path, and an origin may + // delete its own actor. + deleteTarget := h.mat.HandleDelete + if announcer != nil { + deleteTarget = h.mat.HandleDeleteRecord + } + if err := deleteTarget(ctx, targetID); err != nil { return err } return nil } -// handleUndo processes Undo{Like|Dislike|Delete|Follow}. -func (h *Handler) handleUndo(ctx context.Context, undo *ap.Object, signer, announcer string) error { +// announcerGroupID is the announcing community's AP group id, or "" for a +// bare delivery. As a Tombstones scope "" means global/origin-authorized — +// correct for a bare delivery, which reached here only by passing the +// same-authority check against the target id's own host. +func announcerGroupID(announcer *store.Community) string { + if announcer == nil { + return "" + } + return announcer.APGroupID +} + +// handleUndo processes Undo{Like|Dislike|Delete|Follow}. announcer is the +// announcing community (nil when delivered bare). +func (h *Handler) handleUndo(ctx context.Context, undo *ap.Object, signer string, announcer *store.Community) error { inner := undo.Object if inner == nil || inner.Type == "" { // A bare-IRI undo target is unactionable: we cannot know what kind // of activity is being undone without its body. return skip(undo.ID, "undo carries no inline activity") } + announcerID := announcerGroupID(announcer) switch inner.Type { case ap.TypeLike, ap.TypeDislike: // The inbox binds only the OUTER Undo's actor to the signature; the @@ -118,12 +151,12 @@ func (h *Handler) handleUndo(ctx context.Context, undo *ap.Object, signer, annou // any signer could retract other instances' users' votes. Announced // undos ride the announcing community's vouching, exactly like // announced votes (FEP-1b12 group fan-out). - if announcer == "" { + if announcer == nil { if err := h.authorizeBareVote(undo.ID, inner, signer); err != nil { return err } } - return h.votes.RetractVote(ctx, inner, announcer) + return h.votes.RetractVote(ctx, inner, announcerID) case ap.TypeDelete: return h.handleUndoDelete(ctx, undo, inner, signer, announcer) case ap.TypeFollow: @@ -135,30 +168,96 @@ func (h *Handler) handleUndo(ctx context.Context, undo *ap.Object, signer, annou } } -// handleUndoDelete restores an object whose Delete was previously applied: -// clear the create-after-delete marker, re-fetch the object from its origin -// (never trust the undo body), clear the mapping's soft delete, and -// re-materialize. Idempotent; a restore for content that is still gone -// upstream is a skip. -func (h *Handler) handleUndoDelete(ctx context.Context, undo, del *ap.Object, signer, announcer string) error { +// handleUndoDelete undoes a Delete the bridge previously applied. What that +// means depends entirely on what the delete left behind, and the two cases +// are NOT variations of one flow: +// +// - UNMAPPED (a marker and nothing else — the delete-before-create race). +// Nothing was ever materialized, so there is nothing to restore and +// NOTHING is fetched: the undo retracts the marker its own authorization +// context laid and stops (retractDeleteMarker). Fetching here would make +// Undo{Delete{}} an arbitrary-IRI fetch oracle and PLC mint for +// anyone who can lay a marker first — and laying one is cheap by design: +// authorizeDelete deliberately admits an UNMAPPED announced target (that +// allowance is what closes the race at all), and a bare Delete of an id on +// the signer's own host is always authorized. Two activities would +// otherwise buy a mint, bypassing materializeContent's echo, tombstone, +// and community-binding checks entirely. +// - MAPPED. The id IS ours to restore: clear the marker and the mapping's +// soft delete and re-materialize from the object the origin serves again. +// That re-fetch is the authorization ("a restore is only as real as the +// content behind it") AND the record body, so it is pinned to the target's +// own authority, and what comes back must still be the KIND of thing the +// mapping says it is. +// +// Idempotent throughout; a restore for content that is still gone upstream is +// a skip. +// +// Deliberate, operator-visible policy: a BARE restore of mapped content +// overrides a community's moderation delete of that content. It is bounded by +// the same-authority signer, an existing mapping, a pinned re-fetch and the +// type check — i.e. an origin re-serving content it previously bridged — and +// the origin re-serving an object is the strongest statement anyone makes +// about it, which is what the bridge mirrors. +func (h *Handler) handleUndoDelete(ctx context.Context, undo, del *ap.Object, signer string, announcer *store.Community) error { targetID := refID(del.Object) if targetID == "" { return skip(undo.ID, "undone delete carries no object id") } - // Same authorization rule as the delete itself (a restore must not let a - // cross-authority or co-hosted actor un-delete a victim's content). + // Who may restore is who may delete: same rule, so an unrelated instance + // or a community the target does not belong to cannot un-delete a + // victim's content. if err := h.authorizeDelete(ctx, undo.ID, targetID, signer, announcer); err != nil { return err } + scope := announcerGroupID(announcer) - // The origin must actually serve the object again — a restore is only - // as real as the content behind it. - restored, err := h.fetchBound(ctx, targetID) + mapping, err := h.objects.GetByAPID(ctx, targetID) + if errors.IsNotFound(err) { + return h.retractDeleteMarker(ctx, undo.ID, targetID, scope) + } + if err != nil { + return fmt.Errorf("ingest: look up mapping for restore of %s: %w", targetID, err) + } + + // Pinned to the target's own authority: this fetch's answer is what + // authorizes the restore AND what gets written into the repo, so an open + // redirect on the origin must fail it rather than both license the restore + // and choose its content (the delete sweep's fetch is pinned for the first + // half of that reason alone). resolveDelivered's ordinary re-fetch stays + // permissive on purpose: there the origin's answer is content only, and the + // self-asserted-id binding already contains it. + restored, err := h.fetchBoundSameAuthority(ctx, targetID) if err != nil { return err } + // The re-materialization below is HandleUpdate, which dispatches on the + // FETCHED type, not on the mapping: an id now serving a Person where a post + // used to live would mint/refresh an ACTOR off a content restore, and a Page + // where a comment lived would write into a community repo. Only a type + // consistent with the mapping survives. + if !restoredTypeMatches(mapping.Collection, restored.Type) { + return skip(targetID, fmt.Sprintf( + "restored object is a %q but %s is mapped as %s", restored.Type, targetID, mapping.Collection)) + } + // An announced restore is bound to the announcing community exactly like + // announced content (materializeContent's guard) and one notch tighter: the + // restored body must NAME a community, and it must be the announcer's. The + // materializer derives the target community from the object's own audience + // and EnsureCommunity()s it, so letting an EMPTY audience pass would let a + // community vouch for a restore into whatever the object turns out to name + // — that vacuous pass is what let a sibling community revive another + // community's soft-deleted comment. Real Lemmy bodies always carry audience, + // so requiring it costs nothing. + if announcer != nil { + if objCommunity := communityIRIFrom(restored); objCommunity != announcer.APGroupID { + return skip(targetID, fmt.Sprintf( + "restored object names community %q but was announced by %s", + objCommunity, announcer.APGroupID)) + } + } - if err := h.tombstones.Remove(ctx, targetID); err != nil { + if err := h.tombstones.Remove(ctx, targetID, scope); err != nil { return fmt.Errorf("ingest: clear tombstone for %s: %w", targetID, err) } if err := h.objects.Restore(ctx, targetID); err != nil && !errors.IsNotFound(err) { @@ -166,28 +265,74 @@ func (h *Handler) handleUndoDelete(ctx context.Context, undo, del *ap.Object, si } h.logger.Info("object restored upstream; re-materializing", "ap_id", targetID) if _, err = h.mat.HandleUpdate(ctx, restored); err != nil { - // Compensation: the mapping's soft delete is already cleared and its - // record was deleted from the repo. If re-materialization declines the - // object (a skip — nobridge/deleted author/tombstoned ancestor — or a - // validation error), leaving the mapping live would strand it WITHOUT a - // record (downstream parent-lookup/echo/GetByAPID would treat it as - // materialized). Roll back to the pre-undo state: re-soft-delete the - // mapping and re-record the tombstone. - if materialize.IsSkip(err) || errors.IsValidation(err) { - if rerr := h.objects.SoftDelete(ctx, targetID); rerr != nil && !errors.IsNotFound(rerr) { - return fmt.Errorf("ingest: re-soft-delete after declined restore of %s: %w", targetID, rerr) - } - if rerr := h.tombstones.Record(ctx, targetID); rerr != nil { - return fmt.Errorf("ingest: re-record tombstone after declined restore of %s: %w", targetID, rerr) - } - h.logger.Info("restore re-materialization declined; rolled back to deleted state", - "ap_id", targetID, "reason", err) + // Compensation, for EVERY error class. The mapping's soft delete is + // already cleared and its record was deleted from the repo, so leaving + // the mapping live strands it WITHOUT a record (downstream + // parent-lookup/echo/GetByAPID would read it as materialized). A skip or + // validation error strands it immediately; a RETRYABLE failure strands it + // just as permanently once the attempt cap poisons the event, which is + // why the rollback is unconditional. The original error is returned + // unchanged so retryable still retries — and the retry re-runs this whole + // undo, which is idempotent (Remove/Restore again). + if rerr := h.objects.SoftDelete(ctx, targetID); rerr != nil && !errors.IsNotFound(rerr) { + return fmt.Errorf("ingest: re-soft-delete after failed restore of %s: %w", targetID, rerr) + } + // Same scope the delete would have used: the marker this authorization + // context is entitled to lay. A bare undo that cleared other communities' + // markers does not re-create them — it got here on the target id's own + // authority, which outranks their claim regardless of how this ends. + if rerr := h.tombstones.Record(ctx, targetID, scope); rerr != nil { + return fmt.Errorf("ingest: re-record tombstone after failed restore of %s: %w", targetID, rerr) } + h.logger.Info("restore re-materialization failed; rolled back to deleted state", + "ap_id", targetID, "reason", err) return err } return nil } +// restoredTypeMatches reports whether a re-fetched AP object is the kind of +// thing a mapping in the given collection was made from. Actor/community +// profiles map to no AP content type at all, so they never match: a restore +// is a CONTENT path, and un-deleting an actor is the consent machinery's job +// (Delete(Actor) is terminal by design). +func restoredTypeMatches(collection, apType string) bool { + switch collection { + case materialize.CollectionPost: + return apType == ap.TypePage || apType == ap.TypeArticle + case materialize.CollectionComment: + return apType == ap.TypeNote + default: + return false + } +} + +// retractDeleteMarker is Undo{Delete} for an id with no mapping: the delete +// left a tombstone marker and nothing else, so the undo retracts that marker +// and reports a skip — no fetch, no materialization (handleUndoDelete says +// why). Prior state is still required: the marker must be one this +// authorization context can SEE (global, or its own), so a community cannot +// bootstrap off a marker another community laid. What it CLEARS is narrower +// still — an announced undo removes only that community's own row, leaving a +// global origin-authorized marker standing; a bare undo carries the target +// id's own authority and clears the id's markers outright. +func (h *Handler) retractDeleteMarker(ctx context.Context, activityID, targetID, scope string) error { + tombstoned, err := h.tombstones.ExistsFor(ctx, targetID, scope) + if err != nil { + return fmt.Errorf("ingest: tombstone check for %s: %w", targetID, err) + } + if !tombstoned { + return skip(activityID, "restore for an id the bridge never deleted: "+targetID) + } + if err := h.tombstones.Remove(ctx, targetID, scope); err != nil { + return fmt.Errorf("ingest: clear tombstone for %s: %w", targetID, err) + } + h.logger.Info("delete marker retracted upstream; nothing was ever materialized", + "ap_id", targetID, "scope", scope) + return skip(activityID, "restore of an id that was never materialized: "+targetID+ + " (marker retracted; a fresh Create is what re-materializes it)") +} + // authorizeBareVote enforces who may cast (or retract) a BARE, un-announced // vote: the vote's actor must live on the verified signer's authority — host // granularity, the same instance-is-the-trust-unit rule as bare Delete (an @@ -203,36 +348,143 @@ func (h *Handler) authorizeBareVote(activityID string, vote *ap.Object, signer s } // authorizeDelete enforces who may Delete (or Undo{Delete}) a target id. +// announcer is the announcing community (nil for a bare delivery). // // - Bare (unannounced): only the target id's OWN authority may delete it — // the actor removing their content/account, or their instance acting for // them. -// - Announced by a followed community: the target must live on the -// announcing community's own authority (a community moderates only its own -// instance's content). For a target that is itself a bridged ACTOR — the -// terminal DeleteActor scrub — the community may delete only ITSELF, never -// a co-hosted OTHER actor whose bridged presence spans other communities. -func (h *Handler) authorizeDelete(ctx context.Context, activityID, targetID, signer, announcer string) error { - if announcer == "" { +// - Announced by a followed community: the community moderates the content +// posted INTO it, wherever that content is hosted — a jlai.lu author's +// post in a lemmy.world community carries a jlai.lu ap_id, and its +// Delete fans out through the community's Announce (the normal remote- +// author federation shape). MEMBERSHIP in the announcing community, not +// the target's host, is therefore the test: posts commit into the +// community's own repo (mapping.DID answers directly), comments into +// their AUTHOR's repo (their thread root answers for them — see +// authorizeAnnouncedCommentDelete). A target belonging elsewhere +// (another community's content, even co-hosted on the announcer's +// instance) drops. A target that is itself a bridged ACTOR — the +// terminal DeleteActor scrub — may only be the community ITSELF, never a +// person or another community; both a bridged_actors row and the profile +// mapping refuse it, because a bridged actor always has a profile +// mapping (EnsureActor commits rkey "self") but the actor ROW lands +// first, and only checking the mapping would leave a mid-mint actor +// deletable (HandleDelete keys its terminal scrub off that same row). An +// UNMAPPED target is accepted so handleDelete's tombstone marker still +// closes the delete-before-create window (a Delete can be processed +// while its Create is mid-materialization). That allowance is contained +// structurally, not by trust: the marker it lays is SCOPED to the +// announcing community (migration 015), so it suppresses that id only +// where that community's word counts and can no longer pre-suppress +// another community's content for the retention window. And it reaches no +// further: an Undo of such a delete never fetches or materializes the +// unmapped target — it only retracts that same marker (handleUndoDelete). +func (h *Handler) authorizeDelete(ctx context.Context, activityID, targetID, signer string, announcer *store.Community) error { + if announcer == nil { if !ap.SameAuthority(targetID, signer) { return skip(activityID, fmt.Sprintf( "delete of %s signed by cross-authority actor %s", targetID, signer)) } return nil } - if !ap.SameAuthority(targetID, announcer) { + if targetID == announcer.APGroupID { + return nil + } + if _, err := h.actors.GetByAPActorID(ctx, targetID); err == nil { return skip(activityID, fmt.Sprintf( - "announced delete of %s by cross-authority community %s", targetID, announcer)) + "community %s may not delete actor %s", announcer.APGroupID, targetID)) + } else if !errors.IsNotFound(err) { + return fmt.Errorf("ingest: classify delete target %s: %w", targetID, err) } - if targetID != announcer { - isActor, err := h.targetIsBridgedActor(ctx, targetID) - if err != nil { - return err - } - if isActor { - return skip(activityID, fmt.Sprintf( - "community %s may not delete co-hosted actor %s", announcer, targetID)) - } + mapping, err := h.objects.GetByAPID(ctx, targetID) + if errors.IsNotFound(err) { + return nil + } + if err != nil { + return fmt.Errorf("ingest: classify delete target %s: %w", targetID, err) + } + switch mapping.Collection { + case materialize.CollectionActorProfile, materialize.CollectionCommunityProfile: + // Belt-and-braces behind the actor-row check above: a profile mapping + // whose actor row was scrubbed is still an actor, not content. + return skip(activityID, fmt.Sprintf( + "community %s may not delete actor %s", announcer.APGroupID, targetID)) + case materialize.CollectionComment: + return h.authorizeAnnouncedCommentDelete(ctx, activityID, mapping, announcer) + } + if mapping.DID != announcer.DID { + return skip(activityID, fmt.Sprintf( + "announced delete of %s targets a record outside %s's repo", targetID, announcer.APGroupID)) } return nil } + +// authorizeAnnouncedCommentDelete answers community membership for a +// COMMENT, which its mapping cannot: comments commit into their AUTHOR's +// repo (only posts land in the community's), so mapping.DID is the author's +// DID for every comment in every community — comparing it to the announcer's +// repo would drop every announced comment delete there is. The thread is the +// membership signal instead: the materializer guarantees reply.root on every +// comment and the thread's root post lives in the community's own repo, so +// one record read recovers the owning community's DID (the same derivation +// the vote aggregator uses to bind announced votes). +func (h *Handler) authorizeAnnouncedCommentDelete(ctx context.Context, activityID string, mapping *store.APObjectMapping, announcer *store.Community) error { + if mapping.IsDeleted() { + // Already soft-deleted: the record — and with it the reply.root this + // check reads — is gone from the repo, so there is nothing left to + // authorize against, and re-deleting is a downstream no-op. The + // allowance is exactly that and no more: idempotence for re-delivered + // deletes. The RESTORE path rides the same authorizeDelete call and is + // therefore admitted here too, but it is not authorized here — its + // guarantees are post-fetch (handleUndoDelete): the origin must serve + // the object again over a pinned fetch, its type must match the + // mapping, and an announced restore must NAME the announcing community. + // Those are what stop a sibling community from reviving another + // community's soft-deleted comment. + return nil + } + record, _, err := h.records.GetRecord(ctx, mapping.DID, mapping.Collection, mapping.RKey) + if errors.IsNotFound(err) { + // A live mapping with no record is a permanent inconsistency: a retry + // would re-read the same missing record forever and wedge the + // ordering key behind it. Log it and drop the delete. + h.logger.Warn("announced comment delete: live mapping has no record", + "ap_id", mapping.APID, "at_uri", mapping.ATURI) + return skip(activityID, "comment "+mapping.ATURI+" has no record to authorize against") + } + if err != nil { + return fmt.Errorf("ingest: read comment record %s: %w", mapping.ATURI, err) + } + rootDID := replyRootDID(record) + if rootDID == "" { + h.logger.Warn("announced comment delete: record carries no reply.root", + "ap_id", mapping.APID, "at_uri", mapping.ATURI) + return skip(activityID, "comment "+mapping.ATURI+" carries no reply.root to authorize against") + } + if rootDID != announcer.DID { + return skip(activityID, fmt.Sprintf( + "announced delete of %s targets a comment in a thread outside %s", + mapping.APID, announcer.APGroupID)) + } + return nil +} + +// replyRootDID extracts the repo DID from a comment record's reply.root +// strongRef uri (at://did/collection/rkey). Malformed records yield "". +func replyRootDID(record map[string]any) string { + reply, ok := record["reply"].(map[string]any) + if !ok { + return "" + } + root, ok := reply["root"].(map[string]any) + if !ok { + return "" + } + uri, _ := root["uri"].(string) + rest, ok := strings.CutPrefix(uri, "at://") + if !ok { + return "" + } + did, _, _ := strings.Cut(rest, "/") + return did +} diff --git a/internal/ingest/handler.go b/internal/ingest/handler.go index 54674f6..5009df4 100644 --- a/internal/ingest/handler.go +++ b/internal/ingest/handler.go @@ -17,16 +17,26 @@ type Materializer interface { MaterializePost(ctx context.Context, page *ap.Object) (*materialize.Result, error) MaterializeComment(ctx context.Context, note *ap.Object) (*materialize.Result, error) HandleUpdate(ctx context.Context, obj *ap.Object) (*materialize.Result, error) + // HandleDelete branches actor vs content off a FRESH bridged_actors read; + // HandleDeleteRecord is the content-only entry that never can. Callers + // that already classified the target (handleDelete, SweepDeleted) use the + // latter — see materialize.HandleDeleteRecord for the TOCTOU it closes. HandleDelete(ctx context.Context, apID string) error + HandleDeleteRecord(ctx context.Context, apID string) error RefreshActor(ctx context.Context, actorRef *ap.Object) (*store.BridgedActor, error) RefreshCommunity(ctx context.Context, groupRef *ap.Object) (*store.Community, error) EnsureCommunity(ctx context.Context, groupRef *ap.Object) (*store.Community, error) } // Fetcher is the slice of *ap.Client the dispatcher uses to re-fetch -// objects it must not trust from a delivery. +// objects it must not trust from a delivery. The same-authority variant is +// required wherever a fetch's ANSWER is the authorization decision — the +// delete sweep's 410 (sweep.go) and the restore's "the origin serves it +// again" (consent.go) — because a redirect off the object's own origin must +// not be able to answer those. type Fetcher interface { FetchObject(ctx context.Context, iri string) (*ap.Object, error) + FetchObjectSameAuthority(ctx context.Context, iri string) (*ap.Object, error) } // Backfiller is notified when a community's Follow is accepted (the @@ -35,15 +45,25 @@ type Backfiller interface { TriggerAsync(community *store.Community, force bool) } +// RecordGetter is the slice of the repo manager the dispatcher reads +// committed records back through: an announced comment delete is authorized +// by the comment's stored reply.root (see authorizeDelete), which no mapping +// column carries. +type RecordGetter interface { + GetRecord(ctx context.Context, did, collection, rkey string) (record map[string]any, recordCID string, err error) +} + // HandlerOptions configures NewHandler. Materializer, Fetcher, Objects, -// Communities, Tombstones, Votes, and ServiceActorID are required; -// Backfill and Logger are optional. +// Actors, Communities, Tombstones, Records, Votes, and ServiceActorID are +// required; Backfill and Logger are optional. type HandlerOptions struct { Materializer Materializer Fetcher Fetcher Objects store.APObjects + Actors store.BridgedActors Communities store.Communities Tombstones store.Tombstones + Records RecordGetter Votes VoteAggregator Backfill Backfiller // ServiceActorID is the bridge's own AP actor id; Accepts must wrap a @@ -60,8 +80,10 @@ type Handler struct { mat Materializer fetcher Fetcher objects store.APObjects + actors store.BridgedActors communities store.Communities tombstones store.Tombstones + records RecordGetter votes VoteAggregator backfill Backfiller serviceID string @@ -79,12 +101,18 @@ func NewHandler(opts HandlerOptions) (*Handler, error) { if opts.Objects == nil { return nil, errors.NewValidationError("objects", "must not be nil") } + if opts.Actors == nil { + return nil, errors.NewValidationError("actors", "must not be nil") + } if opts.Communities == nil { return nil, errors.NewValidationError("communities", "must not be nil") } if opts.Tombstones == nil { return nil, errors.NewValidationError("tombstones", "must not be nil") } + if opts.Records == nil { + return nil, errors.NewValidationError("records", "must not be nil") + } if opts.Votes == nil { return nil, errors.NewValidationError("votes", "must not be nil") } @@ -99,8 +127,10 @@ func NewHandler(opts HandlerOptions) (*Handler, error) { mat: opts.Materializer, fetcher: opts.Fetcher, objects: opts.Objects, + actors: opts.Actors, communities: opts.Communities, tombstones: opts.Tombstones, + records: opts.Records, votes: opts.Votes, backfill: opts.Backfill, serviceID: opts.ServiceActorID, @@ -133,9 +163,9 @@ func (h *Handler) Process(ctx context.Context, event *store.InboxEvent) error { case ap.TypeCreate, ap.TypeUpdate: return h.handleBareCreateUpdate(ctx, activity, signer) case ap.TypeDelete: - return h.handleDelete(ctx, activity, signer, "") + return h.handleDelete(ctx, activity, signer, nil) case ap.TypeUndo: - return h.handleUndo(ctx, activity, signer, "") + return h.handleUndo(ctx, activity, signer, nil) case ap.TypeAccept: return h.handleAccept(ctx, activity, signer) case ap.TypeReject: @@ -200,9 +230,13 @@ func (h *Handler) handleAnnounce(ctx context.Context, announce *ap.Object, signe case ap.TypeLike, ap.TypeDislike: return h.votes.ApplyVote(ctx, inner, signer) case ap.TypeDelete: - return h.handleDelete(ctx, inner, signer, signer) + // The already-resolved community travels with the activity: the + // delete authorization needs its repo DID, and re-reading it there + // would turn a "we do not follow it" miss into a retryable error on + // an ordering key that would then never drain. + return h.handleDelete(ctx, inner, signer, community) case ap.TypeUndo: - return h.handleUndo(ctx, inner, signer, signer) + return h.handleUndo(ctx, inner, signer, community) default: // Lock, Add, Remove, Block, ... — moderation activities the bridge // does not translate in v1. @@ -250,7 +284,21 @@ func (h *Handler) materializeContent(ctx context.Context, obj *ap.Object, signer // Create-after-delete: a Delete for this id may have arrived before any // materialization (no mapping to tombstone — task 05's known gap). The // ap_tombstones marker closes it here, in the ingest layer. - tombstoned, err := h.tombstones.Exists(ctx, obj.ID) + // + // Markers are scoped to whoever laid them, so the lookup needs this + // delivery's community context — and that context may only come from + // somewhere the DELIVERY cannot choose. Announced: the announcer, which + // the inbox bound to the HTTP signature, so a community's own marker + // suppresses its own re-announce right here, before any outbound fetch. + // Bare: nothing trustworthy names a community yet — the only candidate is + // the delivered body's audience, and a Create carrying a bare reference + // ({"id": X} with no type and no audience) names none at all, which would + // read straight past the community-scoped marker that a delete-before- + // create left for exactly this id. So the early check is global-only + // (still free, and a globally tombstoned id costs no fetch), and the + // community-scoped half runs below against the body resolveDelivered + // actually vouches for. + tombstoned, err := h.tombstones.ExistsFor(ctx, obj.ID, announcer) if err != nil { return fmt.Errorf("ingest: tombstone check for %s: %w", obj.ID, err) } @@ -270,6 +318,18 @@ func (h *Handler) materializeContent(ctx context.Context, obj *ap.Object, signer if communityIRI == "" { return skip(obj.ID, "bare delivery names no community (no audience group IRI)") } + // The community-scoped half of the create-after-delete check, deferred + // from above: this audience comes from a body the origin served (or one + // the signer vouched for on its own authority), not from a reference the + // deliverer wrote, so a marker laid by the community this object claims + // to belong to now applies to it. + tombstoned, err := h.tombstones.ExistsFor(ctx, obj.ID, communityIRI) + if err != nil { + return fmt.Errorf("ingest: tombstone check for %s: %w", obj.ID, err) + } + if tombstoned { + return skip(obj.ID, "object was deleted upstream before it was ever materialized") + } community, err := h.communities.GetByAPGroupID(ctx, communityIRI) if errors.IsNotFound(err) { return skip(obj.ID, "bare delivery for a community we do not follow: "+communityIRI) @@ -328,11 +388,31 @@ func (h *Handler) resolveDelivered(ctx context.Context, obj *ap.Object, signer s } // fetchBound fetches an object by IRI and binds the body's self-asserted id -// to the fetch authority (empty ids inherit the request IRI). Unavailable -// and tombstoned objects are skips: content that cannot be verified at its -// origin is dropped, not retried. +// to the fetch authority (empty ids inherit the request IRI). Redirects stay +// permissive: here the origin's answer is CONTENT, and the id binding below +// is what keeps a redirect from forging another instance's object. func (h *Handler) fetchBound(ctx context.Context, iri string) (*ap.Object, error) { - fetched, err := h.fetcher.FetchObject(ctx, iri) + return h.bindFetch(ctx, iri, h.fetcher.FetchObject) +} + +// fetchBoundSameAuthority is fetchBound with the redirect authority pinned to +// the requested IRI — for the one dispatch fetch whose ANSWER is an +// authorization decision and not just content: the restore's "the origin +// serves this object again" (handleUndoDelete), which both licenses +// re-materializing the record and supplies its body. An open redirect on the +// origin would otherwise hand both to whoever it points at. The refusal is a +// validation error, so the event poisons instead of retrying against a +// redirect the origin is not about to withdraw. +func (h *Handler) fetchBoundSameAuthority(ctx context.Context, iri string) (*ap.Object, error) { + return h.bindFetch(ctx, iri, h.fetcher.FetchObjectSameAuthority) +} + +// bindFetch runs one of the Fetcher's fetches and applies the shared binding +// rules. Unavailable and tombstoned objects are skips: content that cannot be +// verified at its origin is dropped, not retried. +func (h *Handler) bindFetch(ctx context.Context, iri string, + fetch func(context.Context, string) (*ap.Object, error)) (*ap.Object, error) { + fetched, err := fetch(ctx, iri) switch { case err == nil: case errors.IsTombstoned(err): @@ -446,23 +526,6 @@ func (h *Handler) isBridged(ctx context.Context, apID string) (bool, error) { } } -// targetIsBridgedActor reports whether an AP id is a bridged ACTOR (its -// mapping is a profile record), as opposed to a content object. A bridged -// actor always has a profile mapping (EnsureActor commits rkey "self"), so a -// harmful Delete(Actor) against a bridged victim is always detectable here; -// an unbridged actor is a materializer no-op regardless. -func (h *Handler) targetIsBridgedActor(ctx context.Context, apID string) (bool, error) { - mapping, err := h.objects.GetByAPID(ctx, apID) - if errors.IsNotFound(err) { - return false, nil - } - if err != nil { - return false, fmt.Errorf("ingest: classify delete target %s: %w", apID, err) - } - return mapping.Collection == materialize.CollectionActorProfile || - mapping.Collection == materialize.CollectionCommunityProfile, nil -} - // refID returns the id of a possibly-nil object reference. func refID(obj *ap.Object) string { if obj == nil { diff --git a/internal/ingest/handler_test.go b/internal/ingest/handler_test.go index 969e7a9..518bfe8 100644 --- a/internal/ingest/handler_test.go +++ b/internal/ingest/handler_test.go @@ -4,7 +4,9 @@ import ( "context" "net/http" "strings" + "sync" "testing" + "time" "github.com/ipfs/go-cid" "github.com/multiformats/go-multihash" @@ -15,6 +17,7 @@ import ( "tidepool/internal/errors" "tidepool/internal/materialize" "tidepool/internal/store" + "tidepool/internal/testutil" ) // mintCount returns how many identities the fake minter has minted so far. @@ -24,6 +27,45 @@ func (f *fakeMinter) mintCount() int { return f.mints } +// announceCreate delivers Announce{Create{obj}} from a followed community — +// the shape Lemmy fans out — and drains the queue. +func (h *harness) announceCreate(group *remoteActor, activityID string, obj map[string]any) { + h.t.Helper() + require.Equal(h.t, http.StatusAccepted, h.deliver(group, map[string]any{ + "id": activityID, + "type": "Announce", + "actor": group.id, + "audience": group.id, + "object": map[string]any{ + "id": activityID + "/create", + "type": "Create", + "actor": obj["attributedTo"], + "audience": obj["audience"], + "object": obj, + }, + })) + h.drain() +} + +// announceDelete delivers Announce{Delete{targetID}} from a followed +// community, attributed to actor, and drains the queue. +func (h *harness) announceDelete(group *remoteActor, activityID, actor, targetID string) { + h.t.Helper() + require.Equal(h.t, http.StatusAccepted, h.deliver(group, map[string]any{ + "id": activityID, + "type": "Announce", + "actor": group.id, + "audience": group.id, + "object": map[string]any{ + "id": activityID + "/delete", + "type": "Delete", + "actor": actor, + "object": targetID, + }, + })) + h.drain() +} + // followedCommunity registers a second subscribed community (accepted) on an // arbitrary host that can sign inbox deliveries — the attacker's community in // the announced-authorization tests. It skips the WebFinger/Follow lifecycle @@ -49,6 +91,25 @@ func (h *harness) followedCommunity(id, username, instance string) *remoteActor return actor } +// tombstoneAnnouncers lists the raw marker rows for an ap id. ExistsFor +// cannot answer "whose row is it": a global marker is visible in every scope, +// so it masks exactly the per-announcer removals the scoping rules are about. +func (h *harness) tombstoneAnnouncers(apID string) []string { + h.t.Helper() + rows, err := testutil.DB(h.t).Query( + `SELECT announcer FROM ap_tombstones WHERE ap_id = $1 ORDER BY announcer`, apID) + require.NoError(h.t, err) + defer func() { require.NoError(h.t, rows.Close()) }() + announcers := []string{} + for rows.Next() { + var announcer string + require.NoError(h.t, rows.Scan(&announcer)) + announcers = append(announcers, announcer) + } + require.NoError(h.t, rows.Err()) + return announcers +} + // TestAnnounceCreatePageEndToEnd is the definition-of-done flow: subscribe // → Accept → Announce{Create{Page}} delivered by the fake Lemmy → the post // record is visible on the task-04 firehose. @@ -310,7 +371,10 @@ func TestEchoSuppression(t *testing.T) { // TestCreateAfterDeleteTombstone closes the task-05 gap: a Delete arriving // for a never-materialized object must prevent a later Create from -// resurrecting it; Undo{Delete} restores. +// resurrecting it; Undo{Delete} retracts the marker so a fresh Create lands +// again. The undo itself does NOT materialize — there is no mapping to +// restore, and fetching an unmapped id off an undo is the mint oracle +// handleUndoDelete refuses. func TestCreateAfterDeleteTombstone(t *testing.T) { h := newHarness(t) group := h.subscribeTechnology() @@ -323,7 +387,7 @@ func TestCreateAfterDeleteTombstone(t *testing.T) { require.Equal(t, http.StatusAccepted, h.deliver(author, deleteActivity)) h.drain() - tombstoned, err := h.tombstones.Exists(ctx, pageID) + tombstoned, err := h.tombstones.ExistsFor(ctx, pageID, "") require.NoError(t, err) assert.True(t, tombstoned, "a delete of an unseen id must leave a tombstone marker") @@ -334,8 +398,8 @@ func TestCreateAfterDeleteTombstone(t *testing.T) { _, err = h.objects.GetByAPID(ctx, pageID) assert.True(t, errors.IsNotFound(err), "create-after-delete must not resurrect the object") - // 3. Undo{Delete}: the origin restored the post; the bridge re-fetches - // and materializes it. + // 3. Undo{Delete}: the origin retracted the delete. Nothing was ever + // materialized, so the undo only clears the marker. require.Equal(t, http.StatusAccepted, h.deliver(author, map[string]any{ "id": "https://lemmy.world/activities/undo/delete-1", "type": "Undo", @@ -344,11 +408,21 @@ func TestCreateAfterDeleteTombstone(t *testing.T) { })) h.drain() - tombstoned, err = h.tombstones.Exists(ctx, pageID) + tombstoned, err = h.tombstones.ExistsFor(ctx, pageID, "") require.NoError(t, err) assert.False(t, tombstoned, "undo must clear the tombstone marker") + _, err = h.objects.GetByAPID(ctx, pageID) + assert.True(t, errors.IsNotFound(err), + "an undo for an unmapped id restores nothing; only the marker is retracted") + + // 4. ...and with the marker gone, a re-delivered Create materializes the + // post normally — which is how restored-before-first-sight content lands. + announce := loadFixture(t, "announce_create_page_lemmy_world.json") + announce["id"] = "https://lemmy.world/activities/announce/create/after-undo" + require.Equal(t, http.StatusAccepted, h.deliver(group, announce)) + h.drain() mapping, err := h.objects.GetByAPID(ctx, pageID) - require.NoError(t, err, "the restored object must be materialized") + require.NoError(t, err, "a create after the marker is retracted must materialize") assert.False(t, mapping.IsDeleted()) } @@ -559,8 +633,10 @@ func TestForgedEmbeddedContentRefetched(t *testing.T) { } // TestAnnouncedDeleteOfOwnPost (Finding 1, positive): a followed community -// announcing a Delete of its OWN post (same host, same authority) is -// authorized and soft-deletes the record. +// announcing a Delete of its OWN post is authorized and soft-deletes the +// record. (Same host here, but that is incidental: the post is mapped into +// the announcing community's repo, and repo membership — not host authority +// — is what authorizes the delete.) func TestAnnouncedDeleteOfOwnPost(t *testing.T) { h := newHarness(t) group := h.subscribeTechnology() @@ -594,9 +670,10 @@ func TestAnnouncedDeleteOfOwnPost(t *testing.T) { } // TestCrossAuthorityAnnouncedDeleteDropped (Finding 1, negative): a followed -// community on one instance cannot delete content hosted on ANOTHER instance -// by wrapping the Delete in an Announce. The old code skipped all authority -// checks whenever announcer != "". +// community cannot delete content mapped into a DIFFERENT community's repo +// by wrapping the Delete in an Announce. (Authorization is repo membership, +// not host authority: the victim post here is mapped into the technology +// community's repo, so evil.example's announce drops.) func TestCrossAuthorityAnnouncedDeleteDropped(t *testing.T) { h := newHarness(t) group := h.subscribeTechnology() @@ -627,11 +704,234 @@ func TestCrossAuthorityAnnouncedDeleteDropped(t *testing.T) { mapping, err := h.objects.GetByAPID(ctx, pageID) require.NoError(t, err) assert.False(t, mapping.IsDeleted(), "a community must not delete another instance's content") - tombstoned, err := h.tombstones.Exists(ctx, pageID) + tombstoned, err := h.tombstones.ExistsFor(ctx, pageID, evil.id) require.NoError(t, err) assert.False(t, tombstoned, "an unauthorized announced delete must not record a tombstone") } +// TestAnnouncedDeleteOfRemoteAuthorPost: the normal remote-author federation +// shape — a post whose ap_id lives on the AUTHOR's instance, posted into a +// community on another instance, deleted via the community's Announce. The +// old same-authority rule dropped every such delete (prod incident: a +// jlai.lu author's deleted post in fediverse@lemmy.world survived as a +// duplicate); membership in the announcing community's repo authorizes it. +func TestAnnouncedDeleteOfRemoteAuthorPost(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + ctx := context.Background() + + const ( + remotePersonID = "https://sopuli.example/u/remoteAuthor" + remotePageID = "https://sopuli.example/post/777" + ) + h.newRemoteActor(remotePersonID, person(remotePersonID, "remoteAuthor", nil)) + h.serveObject("/post/777", map[string]any{ + "type": "Page", + "id": remotePageID, + "attributedTo": remotePersonID, + "to": []any{groupID, ap.PublicAudience}, + "name": "cross-instance post", + "audience": groupID, + "published": "2026-07-07T03:27:37.028201Z", + }) + + // The community announces the Create; the embedded object is cross- + // authority to the signer, so the bridge re-fetches it from its origin. + require.Equal(t, http.StatusAccepted, h.deliver(group, map[string]any{ + "id": "https://lemmy.world/activities/announce/create/remote-author", + "type": "Announce", + "actor": groupID, + "audience": groupID, + "object": map[string]any{"id": remotePageID}, + })) + h.drain() + mapping, err := h.objects.GetByAPID(ctx, remotePageID) + require.NoError(t, err) + require.False(t, mapping.IsDeleted()) + + // The author deletes; the delete fans out through the community. + require.Equal(t, http.StatusAccepted, h.deliver(group, map[string]any{ + "id": "https://lemmy.world/activities/announce/delete/remote-author", + "type": "Announce", + "actor": groupID, + "audience": groupID, + "object": map[string]any{ + "id": "https://sopuli.example/activities/delete/777", + "type": "Delete", + "actor": remotePersonID, + "object": remotePageID, + }, + })) + h.drain() + + mapping, err = h.objects.GetByAPID(ctx, remotePageID) + require.NoError(t, err) + assert.True(t, mapping.IsDeleted(), + "an announced delete of a remote-author post in the community's own repo must apply") + _, _, err = h.manager.GetRecord(ctx, mapping.DID, mapping.Collection, mapping.RKey) + assert.Error(t, err, "the record must be deleted from the repo") +} + +// TestAnnouncedDeleteBeforeCreateCrossAuthority: the prod race — the Delete +// announce is processed while the Create is still materializing (or before +// it arrives at all). An unmapped cross-authority target must still record +// the tombstone marker so the late Create cannot resurrect the object. +func TestAnnouncedDeleteBeforeCreateCrossAuthority(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + ctx := context.Background() + + const ( + remotePersonID = "https://sopuli.example/u/remoteAuthor" + remotePageID = "https://sopuli.example/post/778" + ) + h.newRemoteActor(remotePersonID, person(remotePersonID, "remoteAuthor", nil)) + h.serveObject("/post/778", map[string]any{ + "type": "Page", + "id": remotePageID, + "attributedTo": remotePersonID, + "to": []any{groupID, ap.PublicAudience}, + "name": "deleted before create landed", + "audience": groupID, + "published": "2026-07-07T03:27:37.028201Z", + }) + + require.Equal(t, http.StatusAccepted, h.deliver(group, map[string]any{ + "id": "https://lemmy.world/activities/announce/delete/early", + "type": "Announce", + "actor": groupID, + "audience": groupID, + "object": map[string]any{ + "id": "https://sopuli.example/activities/delete/778", + "type": "Delete", + "actor": remotePersonID, + "object": remotePageID, + }, + })) + h.drain() + + tombstoned, err := h.tombstones.ExistsFor(ctx, remotePageID, groupID) + require.NoError(t, err) + assert.True(t, tombstoned, + "an announced delete of an unseen cross-authority id must leave a tombstone marker") + + require.Equal(t, http.StatusAccepted, h.deliver(group, map[string]any{ + "id": "https://lemmy.world/activities/announce/create/late", + "type": "Announce", + "actor": groupID, + "audience": groupID, + "object": map[string]any{"id": remotePageID}, + })) + h.drain() + _, err = h.objects.GetByAPID(ctx, remotePageID) + assert.True(t, errors.IsNotFound(err), "create-after-delete must not resurrect the object") +} + +// unseenRemotePost serves a never-materialized cross-authority post addressed +// to the technology community — the delete-before-create target shape — and +// returns its (signing-capable) author and id. +func (h *harness) unseenRemotePost(slug string) (*remoteActor, string) { + h.t.Helper() + const authorID = "https://sopuli.example/u/remoteAuthor" + author := h.newRemoteActor(authorID, person(authorID, "remoteAuthor", nil)) + id := "https://sopuli.example/post/" + slug + h.serveObject("/post/"+slug, map[string]any{ + "type": "Page", + "id": id, + "attributedTo": authorID, + "to": []any{groupID, ap.PublicAudience}, + "name": "deleted before create landed", + "audience": groupID, + "published": "2026-07-07T03:27:37.028201Z", + }) + return author, id +} + +// TestAnnouncedDeleteMarkerIsScopedToAnnouncer is the cross-community +// suppression pin. authorizeDelete admits an announced Delete of an UNMAPPED +// id from ANY followed community (that allowance is what closes the +// delete-before-create race), so an unscoped marker would hand every followed +// community a veto over arbitrary ap_ids — including ids belonging to other +// communities — for the whole retention window. The marker is scoped to its +// announcer instead: it holds in that community's own context and nowhere +// else, so a DIFFERENT community's Create for the same id still materializes. +func TestAnnouncedDeleteMarkerIsScopedToAnnouncer(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + ctx := context.Background() + _, postID := h.unseenRemotePost("779") + + // A second followed community announces a Delete of an id it has nothing + // to do with — never materialized, so nothing but the marker happens. + evil := h.followedCommunity("https://evil.example/c/foo", "foo", "evil.example") + h.announceDelete(evil, "https://evil.example/activities/announce/delete/cross-suppress", + evil.id, postID) + + tombstoned, err := h.tombstones.ExistsFor(ctx, postID, evil.id) + require.NoError(t, err) + require.True(t, tombstoned, "the announcer's own marker is recorded") + tombstoned, err = h.tombstones.ExistsFor(ctx, postID, groupID) + require.NoError(t, err) + assert.False(t, tombstoned, "another community must not see it") + + // The community the post actually belongs to announces its Create. + require.Equal(t, http.StatusAccepted, h.deliver(group, map[string]any{ + "id": "https://lemmy.world/activities/announce/create/779", + "type": "Announce", + "actor": groupID, + "audience": groupID, + "object": map[string]any{"id": postID}, + })) + h.drain() + + mapping, err := h.objects.GetByAPID(ctx, postID) + require.NoError(t, err, + "one community's tombstone marker must not suppress another community's content") + assert.False(t, mapping.IsDeleted()) + // The marker itself is untouched — it is scoped, not discarded. + tombstoned, err = h.tombstones.ExistsFor(ctx, postID, evil.id) + require.NoError(t, err) + assert.True(t, tombstoned) +} + +// TestBareDeleteBeforeCreateSuppressesAnnouncedCreate is the other half of +// the scoping rule, in the same shape: a delete that passed the bare +// same-authority check carries the target id's OWN origin authority, so its +// marker is global and suppresses the late announced Create that the +// community-scoped marker above could not. +func TestBareDeleteBeforeCreateSuppressesAnnouncedCreate(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + ctx := context.Background() + author, postID := h.unseenRemotePost("780") + + require.Equal(t, http.StatusAccepted, h.deliver(author, map[string]any{ + "id": "https://sopuli.example/activities/delete/780", + "type": "Delete", + "actor": author.id, + "object": postID, + })) + h.drain() + + for _, scope := range []string{"", groupID, "https://evil.example/c/foo"} { + tombstoned, err := h.tombstones.ExistsFor(ctx, postID, scope) + require.NoError(t, err) + assert.True(t, tombstoned, "an origin-authorized marker is visible in every scope") + } + + require.Equal(t, http.StatusAccepted, h.deliver(group, map[string]any{ + "id": "https://lemmy.world/activities/announce/create/780", + "type": "Announce", + "actor": groupID, + "audience": groupID, + "object": map[string]any{"id": postID}, + })) + h.drain() + _, err := h.objects.GetByAPID(ctx, postID) + assert.True(t, errors.IsNotFound(err), + "a global marker must still stop the create-after-delete race") +} + // TestAnnouncedActorDeleteOfCoHostedActorDropped (Finding 1, actor path): even // on its own authority, a community may delete only ITSELF, never a co-hosted // OTHER actor whose bridged presence spans other communities (the terminal @@ -808,7 +1108,7 @@ func TestUndoDeleteRollsBackWhenRematerializeSkips(t *testing.T) { mapping, err = h.objects.GetByAPID(ctx, postID) require.NoError(t, err) require.True(t, mapping.IsDeleted()) - tombstoned, err := h.tombstones.Exists(ctx, postID) + tombstoned, err := h.tombstones.ExistsFor(ctx, postID, "") require.NoError(t, err) require.True(t, tombstoned) @@ -832,7 +1132,7 @@ func TestUndoDeleteRollsBackWhenRematerializeSkips(t *testing.T) { mapping, err = h.objects.GetByAPID(ctx, postID) require.NoError(t, err) assert.True(t, mapping.IsDeleted(), "a declined restore must re-soft-delete the mapping") - tombstoned, err = h.tombstones.Exists(ctx, postID) + tombstoned, err = h.tombstones.ExistsFor(ctx, postID, "") require.NoError(t, err) assert.True(t, tombstoned, "a declined restore must retain the tombstone") } @@ -871,3 +1171,781 @@ func TestLateAcceptAfterUnfollowIgnored(t *testing.T) { "a late Accept must not re-subscribe an unfollowed community") assert.Equal(t, 1, h.backfills.count(), "no fresh backfill for an unfollowed community") } + +// TestAnnouncedDeleteOfComment: the comment counterpart of +// TestAnnouncedDeleteOfOwnPost. Comments commit into their AUTHOR's repo (only +// posts land in the community's), so a membership rule read off mapping.DID +// drops EVERY announced comment delete — including this one, where author and +// community share an instance. The thread root, which does live in the +// community's repo, is what authorizes it. +func TestAnnouncedDeleteOfComment(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + h.serveLemmyWorldContent() + ctx := context.Background() + + require.Equal(t, http.StatusAccepted, + h.deliver(group, loadFixture(t, "announce_create_page_lemmy_world.json"))) + h.drain() + + const ( + commenterID = "https://lemmy.world/u/commenter" + commentID = "https://lemmy.world/comment/3001" + ) + h.serveObject("/u/commenter", person(commenterID, "commenter", nil)) + commentDoc := note(commentID, commenterID, pageID, "a comment", "2026-07-07T05:00:00.000000Z") + h.serveObject("/comment/3001", commentDoc) + h.announceCreate(group, "https://lemmy.world/activities/announce/create/3001", commentDoc) + + mapping, err := h.objects.GetByAPID(ctx, commentID) + require.NoError(t, err) + require.Equal(t, materialize.CollectionComment, mapping.Collection) + require.Equal(t, testDIDFor("commenter", "lemmy.world"), mapping.DID, + "comments live in the author's repo — the premise of this test") + require.NotEqual(t, testDIDFor("technology", "lemmy.world"), mapping.DID) + + // The author deletes; Lemmy fans the Delete out through the community. + h.announceDelete(group, "https://lemmy.world/activities/announce/delete/3001", + commenterID, commentID) + + mapping, err = h.objects.GetByAPID(ctx, commentID) + require.NoError(t, err) + assert.True(t, mapping.IsDeleted(), "an announced comment delete must apply") + _, _, err = h.manager.GetRecord(ctx, mapping.DID, mapping.Collection, mapping.RKey) + assert.Error(t, err, "the comment record must be deleted from the author's repo") +} + +// TestAnnouncedDeleteOfRemoteAuthorComment: the same flow with the author on +// ANOTHER instance — the normal federation shape, where neither the comment's +// ap_id host nor its repo belongs to the announcing community. Only the thread +// root ties it to the community. +func TestAnnouncedDeleteOfRemoteAuthorComment(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + h.serveLemmyWorldContent() + ctx := context.Background() + + require.Equal(t, http.StatusAccepted, + h.deliver(group, loadFixture(t, "announce_create_page_lemmy_world.json"))) + h.drain() + + const ( + remoteCommenterID = "https://sopuli.example/u/sopuliCommenter" + remoteCommentID = "https://sopuli.example/comment/9001" + ) + h.serveObject("/u/sopuliCommenter", person(remoteCommenterID, "sopuliCommenter", nil)) + commentDoc := note(remoteCommentID, remoteCommenterID, pageID, + "a cross-instance comment", "2026-07-07T05:10:00.000000Z") + h.serveObject("/comment/9001", commentDoc) + h.announceCreate(group, "https://lemmy.world/activities/announce/create/9001", commentDoc) + + mapping, err := h.objects.GetByAPID(ctx, remoteCommentID) + require.NoError(t, err) + require.Equal(t, testDIDFor("sopuliCommenter", "sopuli.example"), mapping.DID) + + h.announceDelete(group, "https://lemmy.world/activities/announce/delete/9001", + remoteCommenterID, remoteCommentID) + + mapping, err = h.objects.GetByAPID(ctx, remoteCommentID) + require.NoError(t, err) + assert.True(t, mapping.IsDeleted(), + "a remote author's comment in the community's thread must be deletable by the community") +} + +// TestAnnouncedDeleteFromSiblingCommunityDropped pins the tightening: the +// announcer must own the content, not merely share a host with it. A SECOND +// community on the very same instance (lemmy.world) announces deletes of the +// technology community's post and of a comment in its thread — both drop, +// leaving no tombstone. v1's host-authority rule authorized exactly this; a +// "membership OR same host" reading of the new rule would too. +func TestAnnouncedDeleteFromSiblingCommunityDropped(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + h.serveLemmyWorldContent() + ctx := context.Background() + + require.Equal(t, http.StatusAccepted, + h.deliver(group, loadFixture(t, "announce_create_page_lemmy_world.json"))) + h.drain() + + const ( + commenterID = "https://lemmy.world/u/commenter" + commentID = "https://lemmy.world/comment/3002" + ) + h.serveObject("/u/commenter", person(commenterID, "commenter", nil)) + commentDoc := note(commentID, commenterID, pageID, "a comment", "2026-07-07T05:20:00.000000Z") + h.serveObject("/comment/3002", commentDoc) + h.announceCreate(group, "https://lemmy.world/activities/announce/create/3002", commentDoc) + + // A sibling community, co-hosted on lemmy.world, that the bridge also follows. + sibling := h.followedCommunity("https://lemmy.world/c/otherthing", "otherthing", "lemmy.world") + h.announceDelete(sibling, "https://lemmy.world/activities/announce/sibling-delete-post", + commenterID, pageID) + h.announceDelete(sibling, "https://lemmy.world/activities/announce/sibling-delete-comment", + commenterID, commentID) + + postMapping, err := h.objects.GetByAPID(ctx, pageID) + require.NoError(t, err) + assert.False(t, postMapping.IsDeleted(), "a sibling community must not delete another's post") + commentMapping, err := h.objects.GetByAPID(ctx, commentID) + require.NoError(t, err) + assert.False(t, commentMapping.IsDeleted(), "a sibling community must not delete another's comment") + for _, apID := range []string{pageID, commentID} { + tombstoned, err := h.tombstones.ExistsFor(ctx, apID, sibling.id) + require.NoError(t, err) + assert.False(t, tombstoned, "an unauthorized announced delete must not record a tombstone") + } +} + +// TestAnnouncedUndoDeleteOfUnmappedIDDoesNotInject: the delete path admits an +// unmapped announced target (so its tombstone marker can close the +// delete-before-create race), but the RESTORE path must not — otherwise a +// followed community's Undo{Delete{}} is a fetch oracle and DID mint +// that materializes never-seen objects while bypassing the content funnel's +// echo, tombstone, and community-binding checks. +func TestAnnouncedUndoDeleteOfUnmappedIDDoesNotInject(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + + evil := h.followedCommunity("https://evil.example/c/foo", "foo", "evil.example") + + // The attacker-chosen target: a never-seen object on a victim host, served + // with a hit counter so we can prove it is never fetched. + const ( + victimPost = "https://victim.example/post/999" + victimAuthor = "https://victim.example/u/victim" + victimCommunity = "https://victim.example/c/somewhere" + ) + h.serveObject("/u/victim", person(victimAuthor, "victim", nil)) + h.serveObject("/post/999", map[string]any{ + "type": "Page", + "id": victimPost, + "attributedTo": victimAuthor, + "to": []any{ap.PublicAudience}, + "audience": victimCommunity, + "name": "never seen by the bridge", + "source": map[string]any{"content": "body", "mediaType": "text/markdown"}, + "published": "2026-07-07T09:00:00.000000Z", + }) + mintsBefore := h.minter.mintCount() + + require.Equal(t, http.StatusAccepted, h.deliver(evil, map[string]any{ + "id": "https://evil.example/activities/announce/undo-delete-unmapped", + "type": "Announce", + "actor": evil.id, + "audience": evil.id, + "object": map[string]any{ + "id": "https://evil.example/activities/undo/unmapped", + "type": "Undo", + "actor": evil.id, + "object": map[string]any{ + "id": "https://evil.example/activities/delete/unmapped", + "type": "Delete", + "actor": evil.id, + "object": victimPost, + }, + }, + })) + h.drain() + + event, err := h.events.GetEvent(ctx, "https://evil.example/activities/announce/undo-delete-unmapped") + require.NoError(t, err) + assert.NotNil(t, event.ProcessedAt, "the drop is a processed skip, never a retry") + assert.Equal(t, 0, h.hitCount("/post/999"), + "a restore for an id the bridge never deleted must not fetch it") + assert.Equal(t, mintsBefore, h.minter.mintCount(), "no identity minted for an unseen object") + _, err = h.objects.GetByAPID(ctx, victimPost) + assert.True(t, errors.IsNotFound(err), "the unseen object must not be materialized") + _, err = h.communities.GetByAPGroupID(ctx, victimCommunity) + assert.True(t, errors.IsNotFound(err), "the named community must not be bridged") +} + +// TestAnnouncedUndoDeleteIntoAnotherCommunityDropped: prior state is present +// (the community deleted its own post), so the restore clears the fetch gate — +// but the origin now serves the object addressed to a DIFFERENT community. +// Re-materializing would EnsureCommunity() that other community and write the +// record into its repo, so the restore must drop, mirroring the announced- +// content binding check. +func TestAnnouncedUndoDeleteIntoAnotherCommunityDropped(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + h.serveLemmyWorldContent() + ctx := context.Background() + + const ( + postID = "https://lemmy.world/post/70002" + otherCommunity = "https://lemmy.world/c/otherthing" + ) + page := func(audience string) map[string]any { + return map[string]any{ + "type": "Page", + "id": postID, + "attributedTo": personID, + "to": []any{ap.PublicAudience}, + "audience": audience, + "name": "a post that moves", + "source": map[string]any{"content": "body", "mediaType": "text/markdown"}, + "published": "2026-07-07T09:30:00.000000Z", + } + } + // The other community is fetchable (an unfollowed, co-hosted Group): without + // the binding check the restore's re-materialization would EnsureCommunity() + // it — mint its DID and write the post into its repo. + h.serveObject("/c/otherthing", map[string]any{ + "type": "Group", + "id": otherCommunity, + "preferredUsername": "otherthing", + "inbox": otherCommunity + "/inbox", + "published": "2024-01-01T00:00:00.000000Z", + }) + h.serveObject("/post/70002", page(groupID)) + h.announceCreate(group, "https://lemmy.world/activities/announce/create/70002", page(groupID)) + mapping, err := h.objects.GetByAPID(ctx, postID) + require.NoError(t, err) + require.False(t, mapping.IsDeleted()) + + h.announceDelete(group, "https://lemmy.world/activities/announce/delete/70002", personID, postID) + mapping, err = h.objects.GetByAPID(ctx, postID) + require.NoError(t, err) + require.True(t, mapping.IsDeleted()) + + // The origin restores the post, but now addressed to another community. + h.serveObject("/post/70002", page(otherCommunity)) + require.Equal(t, http.StatusAccepted, h.deliver(group, map[string]any{ + "id": "https://lemmy.world/activities/announce/undo-delete/70002", + "type": "Announce", + "actor": groupID, + "audience": groupID, + "object": map[string]any{ + "id": "https://lemmy.world/activities/undo/70002", + "type": "Undo", + "actor": groupID, + "object": map[string]any{ + "id": "https://lemmy.world/activities/delete/70002", + "type": "Delete", + "actor": personID, + "object": postID, + }, + }, + })) + h.drain() + + mapping, err = h.objects.GetByAPID(ctx, postID) + require.NoError(t, err) + assert.True(t, mapping.IsDeleted(), "a restore into another community must not revive the mapping") + tombstoned, err := h.tombstones.ExistsFor(ctx, postID, groupID) + require.NoError(t, err) + assert.True(t, tombstoned, "the tombstone marker survives a dropped restore") + _, err = h.communities.GetByAPGroupID(ctx, otherCommunity) + assert.True(t, errors.IsNotFound(err), "the other community must not be bridged") +} + +// TestBareReferenceCreateCannotDodgeScopedTombstone pins WHERE the +// create-after-delete check may read its community scope from. Markers are +// community-scoped, so the lookup needs a community context — and before the +// object is resolved a BARE delivery offers only one: the delivered body's +// own audience, which the deliverer wrote. A Create carrying nothing but +// {"id": X} names no community at all, so a lookup keyed off that body reads +// global markers only and sails straight past the community-scoped marker a +// delete-before-create left for exactly that id — from ANY signer with a +// valid signature, for content that is not theirs. That is the marker's core +// case, so the scoped half of the check runs after resolveDelivered, against +// the audience the ORIGIN serves. +func TestBareReferenceCreateCannotDodgeScopedTombstone(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + ctx := context.Background() + author, postID := h.unseenRemotePost("781") + + // The community the post belongs to announces its Delete before the Create + // ever lands: a marker in that community's scope, nothing materialized. + h.announceDelete(group, "https://lemmy.world/activities/announce/delete/781", + author.id, postID) + tombstoned, err := h.tombstones.ExistsFor(ctx, postID, groupID) + require.NoError(t, err) + require.True(t, tombstoned, "the delete-before-create marker is the premise") + + // An unrelated instance delivers a bare Create whose object is nothing but + // a reference to the tombstoned id. + mallory := h.newRemoteActor("https://evil.example/u/mallory", + person("https://evil.example/u/mallory", "mallory", nil)) + require.Equal(t, http.StatusAccepted, h.deliver(mallory, map[string]any{ + "id": "https://evil.example/activities/create/dodge", + "type": "Create", + "actor": mallory.id, + "object": map[string]any{"id": postID}, + })) + h.drain() + + event, err := h.events.GetEvent(ctx, "https://evil.example/activities/create/dodge") + require.NoError(t, err) + assert.NotNil(t, event.ProcessedAt, "the drop is a processed skip, never a retry") + _, err = h.objects.GetByAPID(ctx, postID) + assert.True(t, errors.IsNotFound(err), + "a bare-reference create must not dodge the community-scoped marker") + tombstoned, err = h.tombstones.ExistsFor(ctx, postID, groupID) + require.NoError(t, err) + assert.True(t, tombstoned, "the marker itself is untouched") +} + +// TestBareUndoDeleteOfUnmappedActorNeverFetchesOrMints: Delete-then-Undo of +// an id the bridge never bridged is a two-activity mint primitive if the undo +// treats a marker as evidence worth FETCHING on. Both activities here are +// authorized (an instance may always delete an id on its own host), and under +// the old flow the marker satisfied the restore's prior-state gate, the +// target got fetched, and HandleUpdate turned a Person into a PLC mint. An +// unmapped undo restores nothing, so it must reach neither the network nor +// the minter — it only retracts the marker. +func TestBareUndoDeleteOfUnmappedActorNeverFetchesOrMints(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + + mallory := h.newRemoteActor("https://evil.example/u/mallory", + person("https://evil.example/u/mallory", "mallory", nil)) + const ghost = "https://evil.example/u/ghost" + h.serveObject("/u/ghost", person(ghost, "ghost", nil)) + mintsBefore := h.minter.mintCount() + + require.Equal(t, http.StatusAccepted, h.deliver(mallory, map[string]any{ + "id": "https://evil.example/activities/delete/ghost", + "type": "Delete", + "actor": mallory.id, + "object": ghost, + })) + h.drain() + tombstoned, err := h.tombstones.ExistsFor(ctx, ghost, "") + require.NoError(t, err) + require.True(t, tombstoned, "the delete lays the marker the undo would trade on") + + require.Equal(t, http.StatusAccepted, h.deliver(mallory, map[string]any{ + "id": "https://evil.example/activities/undo/ghost", + "type": "Undo", + "actor": mallory.id, + "object": map[string]any{ + "id": "https://evil.example/activities/delete/ghost", + "type": "Delete", + "actor": mallory.id, + "object": ghost, + }, + })) + h.drain() + + assert.Equal(t, 0, h.hitCount("/u/ghost"), + "a restore for an id with no mapping must never fetch its target") + assert.Equal(t, mintsBefore, h.minter.mintCount(), "...and must never mint an identity") + _, err = h.actors.GetByAPActorID(ctx, ghost) + assert.True(t, errors.IsNotFound(err), "the target must not be bridged") + tombstoned, err = h.tombstones.ExistsFor(ctx, ghost, "") + require.NoError(t, err) + assert.False(t, tombstoned, "the undo still retracts the marker it was entitled to lay") +} + +// TestAnnouncedUndoDeleteCannotManufactureRestore is the announced half of +// the same primitive, and the reason the unmapped branch clears markers +// exactly: a followed community can always manufacture "prior state" for an +// arbitrary id (announce Delete{} — the allowance that closes the +// delete-before-create race), so its Undo must buy nothing. No fetch, and no +// authority beyond its own row: the origin-authorized global marker for the +// same id — the one that actually suppresses the id everywhere — survives. +func TestAnnouncedUndoDeleteCannotManufactureRestore(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + + const ( + victimPost = "https://victim.example/post/998" + victimAuthor = "https://victim.example/u/victim" + victimCommunity = "https://victim.example/c/somewhere" + ) + victim := h.newRemoteActor(victimAuthor, person(victimAuthor, "victim", nil)) + h.serveObject("/post/998", map[string]any{ + "type": "Page", + "id": victimPost, + "attributedTo": victimAuthor, + "to": []any{ap.PublicAudience}, + "audience": victimCommunity, + "name": "never seen by the bridge", + "source": map[string]any{"content": "body", "mediaType": "text/markdown"}, + "published": "2026-07-07T09:00:00.000000Z", + }) + mintsBefore := h.minter.mintCount() + + // The victim's own instance deletes the post before the bridge ever saw + // it: an ORIGIN-authorized (global) marker. + require.Equal(t, http.StatusAccepted, h.deliver(victim, map[string]any{ + "id": "https://victim.example/activities/delete/998", + "type": "Delete", + "actor": victimAuthor, + "object": victimPost, + })) + h.drain() + + // An unrelated followed community lays its own marker for that id, then + // undoes it. + evil := h.followedCommunity("https://evil.example/c/foo", "foo", "evil.example") + h.announceDelete(evil, "https://evil.example/activities/announce/delete/998", + evil.id, victimPost) + require.Equal(t, []string{"", evil.id}, h.tombstoneAnnouncers(victimPost), + "both markers coexist (composite key)") + + require.Equal(t, http.StatusAccepted, h.deliver(evil, map[string]any{ + "id": "https://evil.example/activities/announce/undo-delete/998", + "type": "Announce", + "actor": evil.id, + "audience": evil.id, + "object": map[string]any{ + "id": "https://evil.example/activities/undo/998", + "type": "Undo", + "actor": evil.id, + "object": map[string]any{ + "id": "https://evil.example/activities/delete/998", + "type": "Delete", + "actor": evil.id, + "object": victimPost, + }, + }, + })) + h.drain() + + assert.Equal(t, 0, h.hitCount("/post/998"), + "an unmapped restore must not fetch the id it manufactured state for") + assert.Equal(t, mintsBefore, h.minter.mintCount(), "no identity minted") + _, err := h.objects.GetByAPID(ctx, victimPost) + assert.True(t, errors.IsNotFound(err), "the unseen object must not be materialized") + assert.Equal(t, []string{""}, h.tombstoneAnnouncers(victimPost), + "the community clears its OWN row; the origin-authorized marker outranks it") + tombstoned, err := h.tombstones.ExistsFor(ctx, victimPost, evil.id) + require.NoError(t, err) + assert.True(t, tombstoned, "the surviving global marker still suppresses the id everywhere") +} + +// TestAnnouncedRestoreWithoutAudienceDropped: an announced restore must NAME +// the announcing community, not merely fail to contradict it. A body with no +// audience passed the old "empty is fine" binding vacuously — which is how a +// sibling community could revive a comment another community had deleted: +// the comment is already soft-deleted, so the reply.root membership check has +// nothing left to read and admits the activity (idempotence for re-delivered +// deletes), leaving the post-fetch binding as the only guard. Real Lemmy +// bodies always carry audience, so demanding it costs nothing. +func TestAnnouncedRestoreWithoutAudienceDropped(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + h.serveLemmyWorldContent() + ctx := context.Background() + + require.Equal(t, http.StatusAccepted, + h.deliver(group, loadFixture(t, "announce_create_page_lemmy_world.json"))) + h.drain() + + const ( + commenterID = "https://lemmy.world/u/commenter" + commentID = "https://lemmy.world/comment/3003" + ) + h.serveObject("/u/commenter", person(commenterID, "commenter", nil)) + // No audience: comments derive their community from the thread root, so + // this materializes and deletes normally — the field only matters to the + // restore's binding check. + commentDoc := map[string]any{ + "type": "Note", + "id": commentID, + "attributedTo": commenterID, + "to": []any{ap.PublicAudience}, + "content": "

a comment

", + "source": map[string]any{"content": "a comment", "mediaType": "text/markdown"}, + "published": "2026-07-07T05:30:00.000000Z", + "inReplyTo": pageID, + } + h.serveObject("/comment/3003", commentDoc) + h.announceCreate(group, "https://lemmy.world/activities/announce/create/3003", commentDoc) + mapping, err := h.objects.GetByAPID(ctx, commentID) + require.NoError(t, err) + require.False(t, mapping.IsDeleted()) + + // The owning community deletes it... + h.announceDelete(group, "https://lemmy.world/activities/announce/delete/3003", + commenterID, commentID) + mapping, err = h.objects.GetByAPID(ctx, commentID) + require.NoError(t, err) + require.True(t, mapping.IsDeleted()) + + // ...and a co-hosted sibling community the bridge also follows tries to + // bring it back. + sibling := h.followedCommunity("https://lemmy.world/c/otherthing", "otherthing", "lemmy.world") + require.Equal(t, http.StatusAccepted, h.deliver(sibling, map[string]any{ + "id": "https://lemmy.world/activities/announce/sibling-undo-3003", + "type": "Announce", + "actor": sibling.id, + "audience": sibling.id, + "object": map[string]any{ + "id": "https://lemmy.world/activities/undo/sibling-3003", + "type": "Undo", + "actor": sibling.id, + "object": map[string]any{ + "id": "https://lemmy.world/activities/delete/sibling-3003", + "type": "Delete", + "actor": commenterID, + "object": commentID, + }, + }, + })) + h.drain() + + mapping, err = h.objects.GetByAPID(ctx, commentID) + require.NoError(t, err) + assert.True(t, mapping.IsDeleted(), + "a restore whose body names no community must not revive the mapping") + _, _, err = h.manager.GetRecord(ctx, mapping.DID, mapping.Collection, mapping.RKey) + assert.Error(t, err, "the record stays deleted") +} + +// TestAnnouncedRestoreOfChangedTypeDropped: the re-materialization behind a +// restore is HandleUpdate, which dispatches on the FETCHED type rather than +// on the mapping. An id that used to serve a post and now serves a Person +// would therefore turn a content restore into an actor mint/refresh — with no +// audience to fail the community binding, since actor documents carry none. +// The mapping's collection is the invariant: only a consistent type restores. +func TestAnnouncedRestoreOfChangedTypeDropped(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + h.serveLemmyWorldContent() + ctx := context.Background() + + const postID = "https://lemmy.world/post/70004" + page := map[string]any{ + "type": "Page", + "id": postID, + "attributedTo": personID, + "to": []any{ap.PublicAudience}, + "audience": groupID, + "name": "a post that changes shape", + "source": map[string]any{"content": "body", "mediaType": "text/markdown"}, + "published": "2026-07-07T10:30:00.000000Z", + } + h.serveObject("/post/70004", page) + h.announceCreate(group, "https://lemmy.world/activities/announce/create/70004", page) + h.announceDelete(group, "https://lemmy.world/activities/announce/delete/70004", personID, postID) + mapping, err := h.objects.GetByAPID(ctx, postID) + require.NoError(t, err) + require.True(t, mapping.IsDeleted()) + mintsBefore := h.minter.mintCount() + + // The origin now serves an ACTOR at the post's id. + h.serveObject("/post/70004", person(postID, "shapeshifter", nil)) + require.Equal(t, http.StatusAccepted, h.deliver(group, map[string]any{ + "id": "https://lemmy.world/activities/announce/undo-delete/70004", + "type": "Announce", + "actor": groupID, + "audience": groupID, + "object": map[string]any{ + "id": "https://lemmy.world/activities/undo/70004", + "type": "Undo", + "actor": groupID, + "object": map[string]any{ + "id": "https://lemmy.world/activities/delete/70004", + "type": "Delete", + "actor": personID, + "object": postID, + }, + }, + })) + h.drain() + + mapping, err = h.objects.GetByAPID(ctx, postID) + require.NoError(t, err) + assert.True(t, mapping.IsDeleted(), "a type-mismatched restore must not revive the mapping") + assert.Equal(t, mintsBefore, h.minter.mintCount(), "a content restore must never mint an actor") + _, err = h.actors.GetByAPActorID(ctx, postID) + assert.True(t, errors.IsNotFound(err), "the post's id must not become a bridged actor") +} + +// TestUndoDeleteRejectsCrossAuthorityRedirect: the restore's re-fetch IS its +// authorization ("the origin must serve the object again") AND the body that +// goes back into the repo, so an open redirect off the origin would both +// license the restore and choose its content. Pinned like the delete sweep's +// fetch (TestSweepDeletedRejectsCrossAuthorityRedirect), one call over. +func TestUndoDeleteRejectsCrossAuthorityRedirect(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + h.serveLemmyWorldContent() + author := h.newRemoteActor(personID, person(personID, "LeftLeaningFreedomFighters", nil)) + ctx := context.Background() + + const slug = "restore-redirect" + postID := "https://lemmy.world/post/" + slug + h.mux.HandleFunc("GET /post/"+slug, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Location", "https://evil.example/post/attacker-restore") + w.WriteHeader(http.StatusFound) + }) + // The attacker host serves a well-formed post claiming the VICTIM's id, so + // only the redirect pin — not the self-asserted-id binding, which compares + // against the requested IRI — can refuse it. + h.serveObject("/post/attacker-restore", map[string]any{ + "type": "Page", + "id": postID, + "attributedTo": personID, + "to": []any{ap.PublicAudience}, + "audience": groupID, + "name": "ATTACKER RESTORE", + "source": map[string]any{"content": "attacker body", "mediaType": "text/markdown"}, + "published": "2026-07-07T11:00:00.000000Z", + }) + require.Equal(t, postID, h.bridgePost(group, slug)) + + require.Equal(t, http.StatusAccepted, h.deliver(author, map[string]any{ + "id": "https://lemmy.world/activities/delete/" + slug, + "type": "Delete", + "actor": personID, + "object": postID, + })) + h.drain() + mapping, err := h.objects.GetByAPID(ctx, postID) + require.NoError(t, err) + require.True(t, mapping.IsDeleted()) + + require.Equal(t, http.StatusAccepted, h.deliver(author, map[string]any{ + "id": "https://lemmy.world/activities/undo/" + slug, + "type": "Undo", + "actor": personID, + "object": map[string]any{ + "id": "https://lemmy.world/activities/delete/" + slug, + "type": "Delete", + "actor": personID, + "object": postID, + }, + })) + h.drain() + + event, err := h.events.GetEvent(ctx, "https://lemmy.world/activities/undo/"+slug) + require.NoError(t, err) + assert.NotNil(t, event.FailedAt, "an off-authority redirect is permanent, so the event poisons") + assert.Contains(t, event.Error, "authority", + "the refusal must name the authority hop, not some later failure") + mapping, err = h.objects.GetByAPID(ctx, postID) + require.NoError(t, err) + assert.True(t, mapping.IsDeleted(), "a redirected restore must not revive the record") + tombstoned, err := h.tombstones.ExistsFor(ctx, postID, "") + require.NoError(t, err) + assert.True(t, tombstoned, "...nor clear the marker") +} + +// oneShotMissingActors hides ONE ap id from the first bridged_actors lookup +// and delegates every later one: the deterministic stand-in for the mid-mint +// window, where a row is absent when authorization reads it and present when +// the materializer does. +type oneShotMissingActors struct { + store.BridgedActors + apID string + mu sync.Mutex + hid bool +} + +func (s *oneShotMissingActors) GetByAPActorID(ctx context.Context, apID string) (*store.BridgedActor, error) { + if s.hideOnce(apID) { + return nil, errors.NewNotFoundError("bridged_actor", apID) + } + return s.BridgedActors.GetByAPActorID(ctx, apID) +} + +func (s *oneShotMissingActors) hideOnce(apID string) bool { + s.mu.Lock() + defer s.mu.Unlock() + if apID != s.apID || s.hid { + return false + } + s.hid = true + return true +} + +// oneShotMissingObjects is oneShotMissingActors for the mapping table: the +// actor ROW lands before the profile mapping, so a faithful mid-mint window +// hides both from the same first look. +type oneShotMissingObjects struct { + store.APObjects + apID string + mu sync.Mutex + hid bool +} + +func (s *oneShotMissingObjects) GetByAPID(ctx context.Context, apID string) (*store.APObjectMapping, error) { + s.mu.Lock() + first := apID == s.apID && !s.hid + if first { + s.hid = true + } + s.mu.Unlock() + if first { + return nil, errors.NewNotFoundError("ap_object", apID) + } + return s.APObjects.GetByAPID(ctx, apID) +} + +// swapHandlerStores rebuilds the dispatcher (and the queue drain() pumps) +// over substitute store views. The inbox keeps its own wiring: events are +// claimed from the database, so only the processor's identity matters here. +func (h *harness) swapHandlerStores(objects store.APObjects, actors store.BridgedActors) { + h.t.Helper() + handler, err := NewHandler(HandlerOptions{ + Materializer: h.mat, + Fetcher: h.client, + Objects: objects, + Actors: actors, + Communities: h.communities, + Tombstones: h.tombstones, + Records: h.manager, + Votes: h.votes, + Backfill: h.backfills, + ServiceActorID: h.service.ID, + }) + require.NoError(h.t, err) + h.handler = handler + queue, err := NewQueue(QueueOptions{ + Events: h.events, + Processor: handler, + Workers: 1, + MaxAttempts: 3, + Lease: time.Minute, + }) + require.NoError(h.t, err) + h.queue = queue +} + +// TestAnnouncedDeleteCannotScrubMidMintActor closes the actor-vs-content +// TOCTOU. authorizeDelete classifies the target with the rows it can see, and +// during a mint there are none — neither bridged_actors nor the profile +// mapping has landed — so the delete is admitted as an unmapped announced id +// (the allowance that closes the delete-before-create race). If the +// materializer then re-derived the branch from a FRESH bridged_actors read, +// the row that landed in between would turn an unrelated community's content +// delete into that actor's terminal scrub. The classification travels with +// the dispatch instead. +func TestAnnouncedDeleteCannotScrubMidMintActor(t *testing.T) { + h := newHarness(t) + group := h.subscribeTechnology() + h.serveLemmyWorldContent() + ctx := context.Background() + + require.Equal(t, http.StatusAccepted, + h.deliver(group, loadFixture(t, "announce_create_page_lemmy_world.json"))) + h.drain() + actorRow, err := h.actors.GetByAPActorID(ctx, personID) + require.NoError(t, err) + require.Equal(t, store.ConsentStateOK, actorRow.ConsentState) + + h.swapHandlerStores( + &oneShotMissingObjects{APObjects: h.objects, apID: personID}, + &oneShotMissingActors{BridgedActors: h.actors, apID: personID}, + ) + evil := h.followedCommunity("https://evil.example/c/foo", "foo", "evil.example") + h.announceDelete(evil, "https://evil.example/activities/announce/delete/mid-mint", + evil.id, personID) + + actorRow, err = h.actors.GetByAPActorID(ctx, personID) + require.NoError(t, err) + assert.Equal(t, store.ConsentStateOK, actorRow.ConsentState, + "an announced content delete must never reach the terminal actor scrub") + mapping, err := h.objects.GetByAPID(ctx, pageID) + require.NoError(t, err) + assert.False(t, mapping.IsDeleted(), "the actor's content stays live") + _, _, err = h.manager.GetRecord(ctx, actorRow.DID, + materialize.CollectionActorProfile, materialize.ProfileRKey) + assert.NoError(t, err, "the profile record survives too") +} diff --git a/internal/ingest/inbox_test.go b/internal/ingest/inbox_test.go index 239dc99..f9ff77d 100644 --- a/internal/ingest/inbox_test.go +++ b/internal/ingest/inbox_test.go @@ -145,7 +145,7 @@ func TestInboxTombstonedSelfDeleteAccepted(t *testing.T) { h.drain() // Processed as a real delete: the tombstone marker (the // create-after-delete guard) is recorded for the actor id. - gone, err := h.tombstones.Exists(context.Background(), ghost) + gone, err := h.tombstones.ExistsFor(context.Background(), ghost, "") require.NoError(t, err) assert.True(t, gone, "the accepted self-delete must reach handleDelete") } @@ -196,7 +196,7 @@ func TestInboxTombstonedKeyCannotForgeDeleteOfLiveActor(t *testing.T) { "a live actor's self-delete must not be forgeable via a tombstoned keyId") _, err = h.events.GetEvent(context.Background(), activityID) assert.True(t, errors.IsNotFound(err)) - gone, err := h.tombstones.Exists(context.Background(), victim.id) + gone, err := h.tombstones.ExistsFor(context.Background(), victim.id, "") require.NoError(t, err) assert.False(t, gone, "no tombstone may be recorded for the live victim") } @@ -237,7 +237,7 @@ func TestInboxTombstoneConfirmationInconclusiveDeferred(t *testing.T) { _, err = h.events.GetEvent(context.Background(), activityID) assert.True(t, errors.IsNotFound(err), "deferred deliveries must not be enqueued") h.drain() - gone, err := h.tombstones.Exists(context.Background(), ghost) + gone, err := h.tombstones.ExistsFor(context.Background(), ghost, "") require.NoError(t, err) assert.False(t, gone, "no tombstone may be recorded on an inconclusive confirmation") } @@ -265,7 +265,7 @@ func TestInboxTombstoneConfirmationFollowsSameAuthorityRedirect(t *testing.T) { require.Equal(t, http.StatusAccepted, status, "a same-authority redirect to 410 must confirm the tombstone") h.drain() - gone, err := h.tombstones.Exists(context.Background(), ghost) + gone, err := h.tombstones.ExistsFor(context.Background(), ghost, "") require.NoError(t, err) assert.True(t, gone) } @@ -298,7 +298,7 @@ func TestInboxTombstoneConfirmationRejectsCrossAuthorityRedirect(t *testing.T) { _, err = h.events.GetEvent(context.Background(), activityID) assert.True(t, errors.IsNotFound(err), "rejected deliveries must not be enqueued") h.drain() - gone, err := h.tombstones.Exists(context.Background(), ghost) + gone, err := h.tombstones.ExistsFor(context.Background(), ghost, "") require.NoError(t, err) assert.False(t, gone, "no tombstone may be recorded via an off-origin redirect") } diff --git a/internal/ingest/ingest_test.go b/internal/ingest/ingest_test.go index a019701..59eed46 100644 --- a/internal/ingest/ingest_test.go +++ b/internal/ingest/ingest_test.go @@ -288,8 +288,10 @@ func newHarness(t *testing.T) *harness { Materializer: h.mat, Fetcher: h.client, Objects: objects, + Actors: actors, Communities: communities, Tombstones: tombstones, + Records: manager, Votes: h.votes, Backfill: h.backfills, ServiceActorID: h.service.ID, @@ -463,6 +465,28 @@ func (h *harness) serveLemmyWorldContent() { h.serveJSON("/u/LeftLeaningFreedomFighters", personDoc) } +// bridgePost materializes one post under an ap id the caller owns, so the +// test controls what that id's origin path serves. serveJSON's fixture +// handlers always answer 200 and cannot be re-registered, but the +// interesting authorization cases (404, 500, a redirect) are all non-200 — +// and the announce's embedded Page is same-authority, so bridging it needs +// no fetch of the post path at all. +func (h *harness) bridgePost(group *remoteActor, slug string) string { + h.t.Helper() + postID := "https://lemmy.world/post/" + slug + announce := loadFixture(h.t, "announce_create_page_lemmy_world.json") + announce["id"] = "https://lemmy.world/activities/announce/create/" + slug + create := announce["object"].(map[string]any) + create["id"] = "https://lemmy.world/activities/create/" + slug + create["object"].(map[string]any)["id"] = postID + require.Equal(h.t, http.StatusAccepted, h.deliver(group, announce)) + h.drain() + mapping, err := h.objects.GetByAPID(context.Background(), postID) + require.NoError(h.t, err) + require.False(h.t, mapping.IsDeleted()) + return postID +} + // deliver signs and posts an activity to the bridge inbox, returning the // HTTP status. func (h *harness) deliver(actor *remoteActor, activity map[string]any) int { diff --git a/internal/materialize/updates.go b/internal/materialize/updates.go index 6c30f22..8e74adf 100644 --- a/internal/materialize/updates.go +++ b/internal/materialize/updates.go @@ -42,10 +42,12 @@ func (m *Materializer) HandleUpdate(ctx context.Context, obj *ap.Object) (*Resul } } -// HandleDelete processes an AP Delete (or Tombstone) for an object or an -// actor. Actor ids trigger the full Delete(Actor) scrub; object ids delete -// the single record and soft-delete its mapping. Unknown ids are a logged -// no-op (nothing was ever bridged). Idempotent throughout. +// HandleDelete processes an AP Delete (or Tombstone) whose target has NOT +// been classified yet: a known actor id triggers the full Delete(Actor) +// scrub, everything else routes to HandleDeleteRecord. The actor lookup is a +// fresh read, so a caller that already decided the target is content must +// call HandleDeleteRecord directly rather than come through here — see the +// TOCTOU that entry point closes. func (m *Materializer) HandleDelete(ctx context.Context, apID string) error { if apID == "" { return errors.NewValidationError("ap_id", "must not be empty") @@ -58,7 +60,24 @@ func (m *Materializer) HandleDelete(ctx context.Context, apID string) error { } else if !errors.IsNotFound(err) { return fmt.Errorf("materialize: look up actor for delete %s: %w", apID, err) } + return m.HandleDeleteRecord(ctx, apID) +} +// HandleDeleteRecord is the CONTENT-only delete: one record removed, its +// mapping soft-deleted, and never — under any interleaving — the terminal +// Delete(Actor) scrub. That is the point of it existing separately. +// Authorization decides actor-vs-content against the state it can see +// (ingest.authorizeDelete: an announced delete may only ever reach content), +// and an actor mid-mint has NEITHER a bridged_actors row nor a mapping at +// that moment but an actor row moments later; re-deriving the branch here +// from a fresh read would hand that window's actor a terminal scrub off an +// unrelated community's announced delete. Actor/community PROFILE mappings +// are refused for the same reason — a profile record belongs to the actor +// paths. Unknown ids are a logged no-op; idempotent throughout. +func (m *Materializer) HandleDeleteRecord(ctx context.Context, apID string) error { + if apID == "" { + return errors.NewValidationError("ap_id", "must not be empty") + } mapping, err := m.objects.GetByAPID(ctx, apID) if errors.IsNotFound(err) { // Nothing bridged under this id. Usually a delete for content we @@ -73,6 +92,11 @@ func (m *Materializer) HandleDelete(ctx context.Context, apID string) error { if err != nil { return fmt.Errorf("materialize: look up mapping for delete %s: %w", apID, err) } + if mapping.Collection == CollectionActorProfile || mapping.Collection == CollectionCommunityProfile { + m.logger.Warn("content delete targets an actor profile; refused", + "ap_id", apID, "at_uri", mapping.ATURI) + return nil + } return m.deleteMapping(ctx, mapping) } diff --git a/internal/store/hardening_test.go b/internal/store/hardening_test.go index 69a2934..c3af6fd 100644 --- a/internal/store/hardening_test.go +++ b/internal/store/hardening_test.go @@ -2,6 +2,7 @@ package store import ( "context" + "database/sql" "testing" "time" @@ -22,22 +23,29 @@ func TestTombstonesPrune(t *testing.T) { ctx := context.Background() for _, id := range []string{"https://l.test/post/old1", "https://l.test/post/old2", "https://l.test/post/fresh"} { - require.NoError(t, tombstones.Record(ctx, id)) + require.NoError(t, tombstones.Record(ctx, id, "")) } + // A second, community-scoped marker on an aged id: pruning is per + // (ap_id, announcer), so this fresh row must survive its aged sibling. + require.NoError(t, tombstones.Record(ctx, "https://l.test/post/old2", "https://l.test/c/tech")) _, err := database.Exec( - `UPDATE ap_tombstones SET deleted_at = NOW() - INTERVAL '40 days' WHERE ap_id LIKE '%old%'`) + `UPDATE ap_tombstones SET deleted_at = NOW() - INTERVAL '40 days' + WHERE ap_id LIKE '%old%' AND announcer = ''`) require.NoError(t, err) n, err := tombstones.Prune(ctx, time.Now().Add(-30*24*time.Hour)) require.NoError(t, err) assert.Equal(t, int64(2), n) - gone, err := tombstones.Exists(ctx, "https://l.test/post/old1") + gone, err := tombstones.ExistsFor(ctx, "https://l.test/post/old1", "") require.NoError(t, err) assert.False(t, gone, "aged marker pruned") - kept, err := tombstones.Exists(ctx, "https://l.test/post/fresh") + kept, err := tombstones.ExistsFor(ctx, "https://l.test/post/fresh", "") require.NoError(t, err) assert.True(t, kept, "fresh marker survives") + sibling, err := tombstones.ExistsFor(ctx, "https://l.test/post/old2", "https://l.test/c/tech") + require.NoError(t, err) + assert.True(t, sibling, "pruning an aged marker must not take a fresh sibling scope with it") // Nothing left to prune: zero, no error. n, err = tombstones.Prune(ctx, time.Now().Add(-30*24*time.Hour)) @@ -45,6 +53,95 @@ func TestTombstonesPrune(t *testing.T) { assert.Zero(t, n) } +// TestTombstoneScoping pins the cross-community suppression fix (migration +// 015): a marker laid by one announcing community is invisible to every other +// community, only origin-authorized ("") markers are global, and an undo +// clears no more than its own authority covers. +func TestTombstoneScoping(t *testing.T) { + database := testutil.DB(t) + testutil.Truncate(t, database, "ap_tombstones") + tombstones := NewTombstones(database) + ctx := context.Background() + + const ( + apID = "https://l.test/post/contested" + commA = "https://l.test/c/aaa" + commB = "https://l.test/c/bbb" + globalD = "https://l.test/post/origin-deleted" + ) + + // A's announced delete of an id it does not own. + require.NoError(t, tombstones.Record(ctx, apID, commA)) + + seen, err := tombstones.ExistsFor(ctx, apID, commA) + require.NoError(t, err) + assert.True(t, seen, "the announcing community sees its own marker") + seen, err = tombstones.ExistsFor(ctx, apID, commB) + require.NoError(t, err) + assert.False(t, seen, "another community must not see A's marker") + seen, err = tombstones.ExistsFor(ctx, apID, "") + require.NoError(t, err) + assert.False(t, seen, "a caller with no community context sees only global markers") + + // B's independent marker for the same id coexists (composite key), and + // removing it in B's context leaves A's alone. + require.NoError(t, tombstones.Record(ctx, apID, commB)) + require.NoError(t, tombstones.Remove(ctx, apID, commB)) + seen, err = tombstones.ExistsFor(ctx, apID, commA) + require.NoError(t, err) + assert.True(t, seen, "an undo in B's context must not clear A's marker") + + // A community's undo clears its OWN row and nothing else. It must not take + // the GLOBAL marker with it: that one carries the ORIGIN's authority, which + // outranks any community's, and destroying it (the old `announcer IN ('', + // $2)` form) inverted the privilege — one followed community's undo + // un-suppressed an id the object's own instance had said was gone. + require.NoError(t, tombstones.Record(ctx, apID, "")) + require.NoError(t, tombstones.Remove(ctx, apID, commA)) + assert.Equal(t, []string{""}, announcersFor(t, database, apID), + "a scoped remove deletes exactly its own row") + seen, err = tombstones.ExistsFor(ctx, apID, commA) + require.NoError(t, err) + assert.True(t, seen, "the origin-authorized marker survives a community's undo") + + // An origin-authorized marker is global — every community sees it... + require.NoError(t, tombstones.Record(ctx, globalD, "")) + for _, scope := range []string{"", commA, commB} { + seen, err = tombstones.ExistsFor(ctx, globalD, scope) + require.NoError(t, err) + assert.True(t, seen, "a global marker is visible in every scope") + } + // ...and a bare (origin-authorized) undo outranks any community's claim: + // it clears every marker for the id. + require.NoError(t, tombstones.Record(ctx, globalD, commA)) + require.NoError(t, tombstones.Remove(ctx, globalD, "")) + seen, err = tombstones.ExistsFor(ctx, globalD, commA) + require.NoError(t, err) + assert.False(t, seen, "an origin-authorized undo clears community markers too") + + // Removing what is not there is a no-op success. + require.NoError(t, tombstones.Remove(ctx, "https://l.test/post/never", commA)) +} + +// announcersFor lists the raw marker rows for an ap id. ExistsFor cannot +// answer "whose row is it" — a global marker is visible in every scope, so it +// masks exactly the per-announcer removals the scoping rules are about. +func announcersFor(t *testing.T, database *sql.DB, apID string) []string { + t.Helper() + rows, err := database.Query( + `SELECT announcer FROM ap_tombstones WHERE ap_id = $1 ORDER BY announcer`, apID) + require.NoError(t, err) + defer func() { require.NoError(t, rows.Close()) }() + announcers := []string{} + for rows.Next() { + var announcer string + require.NoError(t, rows.Scan(&announcer)) + announcers = append(announcers, announcer) + } + require.NoError(t, rows.Err()) + return announcers +} + func TestCommunitiesFollowRetryBookkeeping(t *testing.T) { database := testutil.DB(t) testutil.Truncate(t, database, "communities") diff --git a/internal/store/interfaces.go b/internal/store/interfaces.go index aa02b59..1b886ee 100644 --- a/internal/store/interfaces.go +++ b/internal/store/interfaces.go @@ -250,16 +250,33 @@ type InboxEvents interface { // without) a materialization — the create-after-delete gap: a Create // delivered after its Delete must not resurrect content the origin removed. // Undo{Delete} removes the marker. +// +// Markers are SCOPED to the authority that laid them (migration 015): the +// announcing community's AP group id, or "" for an origin-authorized marker +// (a bare same-authority Delete, the admin sweep's verified 410) which is +// global. Announced deletes are accepted for ids the bridge has no mapping +// for — that allowance is what closes the delete-before-create race — so an +// UNSCOPED marker would let any one followed community pre-suppress arbitrary +// ids belonging to OTHER communities for the whole retention window. Scoping +// keeps a community's reach inside its own content. type Tombstones interface { - // Record idempotently marks an AP id as deleted upstream. - Record(ctx context.Context, apID string) error - - // Exists reports whether the AP id carries a tombstone marker. - Exists(ctx context.Context, apID string) (bool, error) - - // Remove clears the marker (Undo{Delete}/restore). Removing a missing - // marker is a no-op success. - Remove(ctx context.Context, apID string) error + // Record idempotently marks an AP id as deleted upstream, scoped to + // announcer (the announcing community's AP group id; "" for an + // origin-authorized, global marker). + Record(ctx context.Context, apID, announcer string) error + + // ExistsFor reports whether the AP id carries a marker VISIBLE in + // communityIRI's context: a global marker, or one laid by that same + // community. A caller with no community context passes "" and sees only + // global markers. + ExistsFor(ctx context.Context, apID, communityIRI string) (bool, error) + + // Remove clears markers (Undo{Delete}/restore). A community clears its + // OWN marker only — never the global one, which is origin-authorized and + // outranks it; "" is that origin authority and clears every marker for the + // id (see the implementation for why the read and write sides are + // deliberately asymmetric). Removing a missing marker is a no-op success. + Remove(ctx context.Context, apID, communityIRI string) error // Prune deletes markers recorded before the cutoff, in batches, and // returns how many were deleted. Retention trade-off, accepted: a diff --git a/internal/store/tombstones.go b/internal/store/tombstones.go index f7bb303..8196b22 100644 --- a/internal/store/tombstones.go +++ b/internal/store/tombstones.go @@ -18,39 +18,69 @@ func NewTombstones(db *sql.DB) Tombstones { return &postgresTombstones{db: db} } -func (r *postgresTombstones) Record(ctx context.Context, apID string) error { +func (r *postgresTombstones) Record(ctx context.Context, apID, announcer string) error { if apID == "" { return errors.NewValidationError("ap_id", "must not be empty") } + // Per (ap_id, announcer): re-recording keeps the FIRST deleted_at, so a + // re-delivered Delete cannot walk its own marker past the retention + // horizon. Two communities' markers for the same id are separate rows. query := ` - INSERT INTO ap_tombstones (ap_id) - VALUES ($1) - ON CONFLICT (ap_id) DO NOTHING` - if _, err := r.db.ExecContext(ctx, query, apID); err != nil { - return fmt.Errorf("record tombstone %q: %w", apID, err) + INSERT INTO ap_tombstones (ap_id, announcer) + VALUES ($1, $2) + ON CONFLICT (ap_id, announcer) DO NOTHING` + if _, err := r.db.ExecContext(ctx, query, apID, announcer); err != nil { + return fmt.Errorf("record tombstone %q (announcer %q): %w", apID, announcer, err) } return nil } -func (r *postgresTombstones) Exists(ctx context.Context, apID string) (bool, error) { +func (r *postgresTombstones) ExistsFor(ctx context.Context, apID, communityIRI string) (bool, error) { if apID == "" { return false, errors.NewValidationError("ap_id", "must not be empty") } + // '' is always visible (the origin-authorized marker); a community sees + // its own on top of that. A caller with no community context passes '' + // and therefore sees only global markers — the announcer IN (...) form + // collapses to announcer = '' for it, which is exactly the rule. var exists bool - query := `SELECT EXISTS (SELECT 1 FROM ap_tombstones WHERE ap_id = $1)` - if err := r.db.QueryRowContext(ctx, query, apID).Scan(&exists); err != nil { - return false, fmt.Errorf("check tombstone %q: %w", apID, err) + query := `SELECT EXISTS ( + SELECT 1 FROM ap_tombstones WHERE ap_id = $1 AND announcer IN ('', $2))` + if err := r.db.QueryRowContext(ctx, query, apID, communityIRI).Scan(&exists); err != nil { + return false, fmt.Errorf("check tombstone %q (community %q): %w", apID, communityIRI, err) } return exists, nil } -func (r *postgresTombstones) Remove(ctx context.Context, apID string) error { +func (r *postgresTombstones) Remove(ctx context.Context, apID, communityIRI string) error { if apID == "" { return errors.NewValidationError("ap_id", "must not be empty") } + if communityIRI == "" { + // Origin authority (a bare Undo{Delete}, authorized against the target + // id's own host): the id's own instance says the content is live, which + // outranks any community's claim that it is gone. Leaving a + // community-scoped marker here would suppress content the origin is + // serving and the bridge is about to materialize — an inconsistency, + // not a safety margin. Deliberately asymmetric with ExistsFor(''), + // which stays conservative because a READ carries no such proof. + if _, err := r.db.ExecContext(ctx, + `DELETE FROM ap_tombstones WHERE ap_id = $1`, apID); err != nil { + return fmt.Errorf("remove tombstones %q: %w", apID, err) + } + return nil + } + // A community's undo clears that community's OWN row and nothing else — + // not the global marker, which is ORIGIN-authorized and outranks it. The + // earlier `announcer IN ('', $2)` form inverted the privilege: any followed + // community could destroy an origin-authorized marker (and the + // compensation re-Record would then permanently downgrade it to that + // community's scope), so one community's undo could un-suppress content the + // object's own instance said was gone. if _, err := r.db.ExecContext(ctx, - `DELETE FROM ap_tombstones WHERE ap_id = $1`, apID); err != nil { - return fmt.Errorf("remove tombstone %q: %w", apID, err) + `DELETE FROM ap_tombstones WHERE ap_id = $1 AND announcer = $2`, + apID, communityIRI); err != nil { + return fmt.Errorf("remove tombstone %q (community %q): %w", apID, communityIRI, err) } return nil } @@ -62,9 +92,12 @@ const tombstonePruneBatchSize = 1000 func (r *postgresTombstones) Prune(ctx context.Context, cutoff time.Time) (int64, error) { var total int64 for { + // Keyed on the whole (ap_id, announcer) pair: pruning by ap_id alone + // would take a sibling community's still-fresh marker down with an + // aged one. res, err := r.db.ExecContext(ctx, ` - DELETE FROM ap_tombstones WHERE ap_id IN ( - SELECT ap_id FROM ap_tombstones WHERE deleted_at < $1 LIMIT $2 + DELETE FROM ap_tombstones WHERE (ap_id, announcer) IN ( + SELECT ap_id, announcer FROM ap_tombstones WHERE deleted_at < $1 LIMIT $2 )`, cutoff, tombstonePruneBatchSize) if err != nil { return total, fmt.Errorf("prune tombstones before %s: %w", cutoff.Format(time.RFC3339), err) diff --git a/internal/votes/e2e_test.go b/internal/votes/e2e_test.go index 7af5c47..b5f7380 100644 --- a/internal/votes/e2e_test.go +++ b/internal/votes/e2e_test.go @@ -52,6 +52,11 @@ func (s *stubMaterializer) HandleDelete(context.Context, string) error { return nil } +func (s *stubMaterializer) HandleDeleteRecord(context.Context, string) error { + s.t.Fatal("votes must never reach HandleDeleteRecord") + return nil +} + func (s *stubMaterializer) RefreshActor(context.Context, *ap.Object) (*store.BridgedActor, error) { s.t.Fatal("votes must never reach RefreshActor") return nil, nil @@ -76,6 +81,10 @@ func (s *stubFetcher) FetchObject(_ context.Context, iri string) (*ap.Object, er return nil, errors.NewNotFoundError("object", iri) } +func (s *stubFetcher) FetchObjectSameAuthority(ctx context.Context, iri string) (*ap.Object, error) { + return s.FetchObject(ctx, iri) +} + // deliverVote runs one activity through Handler.Process the way the queue // worker does (signature verification and dedupe happened at the inbox; the // handler receives the verified payload + bound actor). @@ -137,8 +146,10 @@ func TestFakeLemmyVoteE2E(t *testing.T) { Materializer: &stubMaterializer{t: t}, Fetcher: &stubFetcher{t: t}, Objects: objects, + Actors: store.NewBridgedActors(database), Communities: communities, Tombstones: store.NewTombstones(database), + Records: &fakeRecords{records: map[string]map[string]any{}}, Votes: agg, ServiceActorID: e2eServiceID, }) @@ -186,8 +197,10 @@ func TestBareVoteDispatch(t *testing.T) { Materializer: &stubMaterializer{t: t}, Fetcher: &stubFetcher{t: t}, Objects: objects, + Actors: store.NewBridgedActors(database), Communities: store.NewCommunities(database), Tombstones: store.NewTombstones(database), + Records: &fakeRecords{records: map[string]map[string]any{}}, Votes: agg, ServiceActorID: e2eServiceID, })