diff --git a/cmd/tidepool/main.go b/cmd/tidepool/main.go --- a/cmd/tidepool/main.go +++ b/cmd/tidepool/main.go @@ -315,7 +315,10 @@ Minter: mintGate, Votes: voteAggregator, // Restoring a NATIVE post reads its pinned CID from here: the author's // repo is not one this bridge hosts. - OutboundObjects: store.NewOutboundObjects(database), + OutboundObjects: store.NewOutboundObjects(database), + // An inbound moderation decision updates the admissions ledger too, so + // the operator surface reflects it when the MODERATOR acts. + Ledger: accept.NewAdmissions(database), ServiceDID: serviceDID, ProfileRefreshTTL: cfg.ProfileRefreshTTL, MaxBlobBytes: cfg.MaxBlobBytes, diff --git a/internal/accept/admissions.go b/internal/accept/admissions.go --- a/internal/accept/admissions.go +++ b/internal/accept/admissions.go @@ -104,6 +104,56 @@ } return nil } +// RecordRemoval marks a post removed BY ITS COMMUNITY, with the removal +// record's own code. It satisfies materialize.ModerationLedger, so the inbound +// moderation path can keep the ledger honest without importing this package. +// +// Why the ledger must learn about it at all: the community repo's removal +// record is the source of truth, but the ledger is what an operator reads when +// an author asks why their post is gone, and what decide()'s per-community rate +// cap counts (a removed post must stop consuming the author's quota). +// +// It is a NARROW UPDATE, never Record(): that upsert rewrites every column, +// including evaluated_snapshot, and blanking that would make the post +// permanently unreadmittable — a moderation action must not destroy the state a +// later readmit needs. A post with no ledger row is left alone: the engine +// writes one for every native post it decides on, so a miss means this post is +// not the acceptance engine's business. +func (a *Admissions) RecordRemoval(ctx context.Context, communityDID, postURI, authorDID, code string) error { + return a.markModeration(ctx, "record removal", communityDID, postURI, authorDID, + StatusRemoved, code, "") +} + +// RecordRestore marks a post accepted again after a moderator restore, pinning +// the CID the fresh acceptance was written against. Same narrow-update contract +// as RecordRemoval. +func (a *Admissions) RecordRestore(ctx context.Context, communityDID, postURI, authorDID, cid string) error { + return a.markModeration(ctx, "record restore", communityDID, postURI, authorDID, + StatusAccepted, "", cid) +} + +func (a *Admissions) markModeration(ctx context.Context, op, communityDID, postURI, authorDID, status, code, acceptedCID string) error { + if communityDID == "" || postURI == "" { + return errors.NewValidationError("admission", "community_did and post_uri are required") + } + // accepted_cid moves only on a restore (the empty string leaves it alone), + // so a removal keeps naming the version that was accepted when it was + // removed — the same pin the removal record carries. + _, err := a.db.ExecContext(ctx, ` + UPDATE admissions + SET status = $3, + decision_code = $4, + author_did = COALESCE(NULLIF($5, ''), author_did), + accepted_cid = COALESCE(NULLIF($6, ''), accepted_cid), + updated_at = now() + WHERE community_did = $1 AND post_uri = $2`, + communityDID, postURI, status, code, authorDID, acceptedCID) + if err != nil { + return fmt.Errorf("accept: %s %s/%s: %w", op, communityDID, postURI, err) + } + return nil +} + // Get returns the admission for a (community, post), or an error satisfying // errors.IsNotFound when the engine has never decided on it. func (a *Admissions) Get(ctx context.Context, communityDID, postURI string) (*Admission, error) { diff --git a/internal/accept/engine.go b/internal/accept/engine.go --- a/internal/accept/engine.go +++ b/internal/accept/engine.go @@ -83,6 +83,20 @@ // what an edit against a standing removal is allowed to do. DecisionModeratorRemoved = "moderator-removed" ) +// ErrModeratorRemovalStands reports that an edit was refused because the +// COMMUNITY has removed the post. It is a DECISION, not a failure: the ledger +// row is written, nothing is enqueued, and the live consume path treats it as +// handled. It exists so the operator surfaces cannot report the edit as +// accepted — Readmit maps it to a removed result, and without it the same call +// that records "removed / moderator-removed" answers 200 accepted/enqueued. +var ErrModeratorRemovalStands = stderrors.New("accept: a moderator removal stands") + +// errRemovalVanished reports that the removal AcceptSubject refused against was +// gone by the time its code was read — the moderators restored the post inside +// the window. It is RETRYABLE and self-healing: the retry's AcceptSubject finds +// no removal and admits the edit normally. +var errRemovalVanished = stderrors.New("accept: the standing removal vanished before its code could be read") + // RemovalCodeAdmissionRevoked is the removal `code` written when a post that WAS // accepted fails RE-admission (an edit made it titleless or over the cap). // @@ -303,7 +317,17 @@ EvaluatedSnapshot: e.evaluatedSnapshot(commit), }) } - return e.accept(ctx, did, communityDID, postURI, commit) + if err := e.accept(ctx, did, communityDID, postURI, commit); err != nil { + if stderrors.Is(err, ErrModeratorRemovalStands) { + // Decided and recorded inside accept(): the community removed this + // post, so the edit does not re-enter it. Nothing is owed on the + // live path — erroring here would redrive the commit forever + // against a removal that is terminal by design. + return nil + } + return err + } + return nil } // The AP op strings the deterministic activity id and the Page translation key @@ -553,7 +577,7 @@ } if code != RemovalCodeAdmissionRevoked { e.logger.Info("edit against a standing moderator removal: acceptance refused, nothing enqueued", "community_did", communityDID, "post", postURI, "removal_code", code) - return e.admissions.Record(ctx, Admission{ + if rerr := e.admissions.Record(ctx, Admission{ AuthorDID: did, CommunityDID: communityDID, PostURI: postURI, @@ -561,7 +585,14 @@ Status: StatusRemoved, DecisionCode: DecisionModeratorRemoved, EvaluatedCID: commit.CID, EvaluatedSnapshot: e.evaluatedSnapshot(commit), - }) + }); rerr != nil { + return rerr + } + // The decision is complete; the sentinel only tells the CALLER what was + // decided. AdmitPost swallows it (nothing is owed on the live path); + // Readmit reports it, so the admin surface stops claiming an acceptance + // this very call refused to write. + return ErrModeratorRemovalStands } if _, rerr := acceptrec.Restore(ctx, e.repos, communityDID, postURI, commit.CID, publishedAtOf(commit.Record), sideEffect); rerr != nil { @@ -571,19 +602,29 @@ return nil } // standingRemovalCode reads the `code` off the removal AcceptSubject refused -// against. A removal that has vanished between the refusal and this read is a -// genuine race — the moderators restored the post in the window — and returns -// an error so the event RETRIES: the retry's AcceptSubject finds no removal and -// admits the edit normally. Treating the miss as "not ours, terminal" would -// strand a post whose removal no longer exists. +// against. The two ways this read can fail are different events and are +// reported differently: // -// An unreadable code is treated as a moderator's, i.e. terminal. The direction -// is deliberate: the recoverable mistake is refusing an edit, and the -// unrecoverable one is pushing a removed post back at its community. +// - NOT FOUND is the benign race: the moderators restored the post between +// the refusal and this read. errRemovalVanished says so, and one retry +// resolves it — the retry's AcceptSubject finds no removal and admits the +// edit. Reporting it as "not ours, terminal" would strand a post whose +// removal no longer exists. +// - ANYTHING ELSE is an infrastructure failure (the repo store is down, a +// timeout). It propagates as itself, so the retry budget is spent on a +// message about a broken read rather than about a "standing removal" that +// was never the problem. +// +// A record whose `code` is absent or not a string yields "", which the caller +// treats as a moderator's — terminal. That direction is deliberate: refusing an +// edit is recoverable, and pushing a removed post back at its community is not. func (e *Engine) standingRemovalCode(ctx context.Context, communityDID, postURI string) (string, error) { rkey := acceptrec.SubjectRKey(postURI) record, _, err := e.repos.GetRecord(ctx, communityDID, acceptrec.CollectionRemoval, rkey) - if err != nil { + switch { + case errors.IsNotFound(err): + return "", fmt.Errorf("%w: %s in %s", errRemovalVanished, postURI, communityDID) + case err != nil: return "", fmt.Errorf("accept: read standing removal %s/%s/%s: %w", communityDID, acceptrec.CollectionRemoval, rkey, err) } @@ -918,6 +959,14 @@ } // Passes now: write/repin the acceptance and enqueue the Page (reusing accept()). if err := e.accept(ctx, did, communityDID, postATURI, commit); err != nil { + if stderrors.Is(err, ErrModeratorRemovalStands) { + // Admission passes, but the COMMUNITY removed this post: accept() + // wrote the removed ledger row and refused the acceptance. Reporting + // it as accepted/enqueued would have the same request answer 200 + // "accepted" while the row it just wrote says removed. + return &ReadmitResult{PostURI: postATURI, Status: StatusRemoved, + DecisionCode: DecisionModeratorRemoved}, nil + } return nil, err } return &ReadmitResult{PostURI: postATURI, Status: StatusAccepted, Enqueued: true}, nil diff --git a/internal/db/migrations/024_ap_object_community_backfill.sql b/internal/db/migrations/024_ap_object_community_backfill.sql --- a/internal/db/migrations/024_ap_object_community_backfill.sql +++ b/internal/db/migrations/024_ap_object_community_backfill.sql @@ -10,6 +10,11 @@ -- announced removal — the accident that has been standing in for authorization. -- The enqueuer now carries community_did on the mapping it writes; this -- backfills the rows written before it did. -- +-- The statement is SAFE TO RE-RUN, by design: it only ever fills rows that have +-- no binding, so an operator who suspects a binding was NULLed (see the +-- COALESCE in store.putMapping, which now prevents that) can replay it verbatim +-- to recover from outbound_objects. +-- -- The backfill is an EXACT JOIN, not a derivation. outbound_objects is the -- bridge's own record of what it federated where, keyed by the same at-uri, and -- it already carries community_did. Guessing (say, from the acceptance records @@ -21,7 +26,9 @@ SET community_did = o.community_did FROM outbound_objects o WHERE a.at_uri = o.at_uri AND a.origin = 'bridge' - AND a.community_did IS NULL + -- IS NULL is the state a row is written in; the COALESCE also covers a row + -- that somehow holds an empty string, so a re-run heals both. + AND COALESCE(a.community_did, '') = '' AND o.community_did <> ''; -- +goose Down diff --git a/internal/ingest/consent.go b/internal/ingest/consent.go --- a/internal/ingest/consent.go +++ b/internal/ingest/consent.go @@ -21,6 +21,7 @@ package ingest import ( "context" + "expvar" "fmt" "tidepool/internal/ap" @@ -169,6 +170,17 @@ // all the PRE-FLIP era, whose posts live in the community's own repo with no // acceptance to replace. Writing a removal for a legacy post would announce a // visibility mechanism Coves does not consult for that collection, so those // keep the v1 behaviour exactly: delete the record, tombstone the mapping. +// +// A NATIVE (bridge-origin) comment is the one case it TAKES without acting: +// declining would run that v1 behaviour against a record in the author's own +// repo. See the branch below. +// NativeCommentModerationDeferred counts announced deletes of NATIVE comments +// that were taken and deliberately not acted on, pending 17c-2's comment +// removal record. It is a DECIDED non-action, so it is counted: the alternative +// reading of a flat zero is "no community has ever tried", and the two must not +// look the same when the feature lands. +var NativeCommentModerationDeferred = expvar.NewInt("tidepool_moderation_native_comment_deferred") + func (h *Handler) moderateAnnouncedDelete(ctx context.Context, del *ap.Object, targetID string) (bool, error) { mapping, err := h.objects.GetByAPID(ctx, targetID) if errors.IsNotFound(err) { @@ -185,7 +197,38 @@ } if err != nil { return false, fmt.Errorf("ingest: look up mapping for removal of %s: %w", targetID, err) } - if mapping.IsDeleted() || mapping.Collection != materialize.CollectionPostV2 { + if mapping.IsDeleted() { + return false, nil + } + if mapping.Collection != materialize.CollectionPostV2 { + // A NATIVE comment: moderation of it is TAKEN here and deferred, never + // declined into the path below. + // + // Declining used to be safe by accident — a bridge-origin mapping had no + // community_did, so authorization refused before reaching this function + // at all. Task 17c binds those mappings, so an announced removal of a + // native comment now arrives authorized, and falling through would run + // the v1 DELETE path on it: soft-delete our own mapping and attempt a + // record delete in the AUTHOR's repo, which this bridge does not host. + // That is destruction on behalf of a decision (comment removal) that + // decision 18 does not authorize, and it is self-inconsistent besides — + // resolveSubject reads the still-live outbound row, so replies to the + // "removed" comment keep federating regardless. + // + // The removal RECORD for comments needs the lexicon work in 17c-2; until + // then the honest outcome is a visible non-action. + // + // EVERY announced delete of a native comment is taken, not only a + // summary-bearing one: the author's own deletes arrive through the + // consumer (their repo), never announced back at us, so an announced + // one is either a moderation action or an echo — and neither may reach + // a path that deletes the author's record. + if mapping.Origin == store.OriginBridge { + NativeCommentModerationDeferred.Add(1) + return true, skip(targetID, + "announced delete of a native comment: moderation of native comments is not "+ + "implemented yet (needs the 17c-2 removal record), taking no action") + } return false, nil } if !del.HasSummary() { @@ -321,6 +364,24 @@ if err != nil { return fmt.Errorf("ingest: look up mapping for restore of %s: %w", targetID, err) } + // OUR OWN CONTENT TAKES THE MODERATION PATH, NEVER THE RE-MATERIALIZATION + // ONE — decided BEFORE the fetch, because the fetch is the first step of + // the damage. + // + // A bridge-origin id is a record in a NATIVE author's repo that this bridge + // federated outward. There is nothing to re-materialize: the removal never + // touched the record, only the community's acceptance of it, so a restore is + // that acceptance coming back and nothing else. Falling through would + // dereference our own origin, hand the result to HandleUpdate (which never + // sees materializeContent's bridge-origin echo guard), MINT a bridged actor + // and PLC DID for a native Coves user, fail the commit against a repo we do + // not host, and then compensate by soft-deleting our own mapping and + // tombstoning our own AP id — after which moderateAnnouncedDelete declines + // forever on IsDeleted() and the post can never be moderated again. + if mapping.Origin == store.OriginBridge { + return h.restoreNativeContent(ctx, undo, mapping, announcer, scope) + } + // 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 @@ -437,6 +498,56 @@ return apType == ap.TypeNote default: return false } +} + +// restoreNativeContent lifts a community's removal of a post THIS BRIDGE +// federated on a native author's behalf. It is the mirror of +// moderateAnnouncedDelete: the same actor (the owning community), the same +// records (its acceptance and removal, one rkey, one commit), and the same +// hands-off rule about the author's record, which neither the removal nor the +// restore ever touches. +// +// Only an ANNOUNCED undo may do it. A bare Undo{Delete} would have to come from +// the target id's own authority, and that authority is US — a bare restore of +// our own object is either an echo of something we never send or somebody +// claiming to speak for our origin, and neither is a moderation decision. +// +// The two state clears are legacy repair, not part of the restore: an announced +// delete of native content in the pre-17c era laid a marker and soft-deleted the +// mapping (there was no community binding, so it fell into the v1 delete path). +// Both are idempotent and cost one statement each, and leaving either behind +// would keep the post suppressed or unmoderatable after a legitimate restore. +// +// It is deliberately NOT gated on the undone Delete's `summary`. On the delete +// side that key separates two opposite actions — destroy the author's record, +// or record a community removal — so presence has to decide. Here both readings +// converge on the same non-destructive outcome (the acceptance returns), and +// requiring the key would only create a way for a real restore to be dropped, +// leaving a removal the moderators lifted standing forever. +func (h *Handler) restoreNativeContent(ctx context.Context, undo *ap.Object, + mapping *store.APObjectMapping, announcer *store.Community, scope string) error { + + if announcer == nil { + return skip(mapping.APID, "bare undo of a delete cannot restore the bridge's own content") + } + if mapping.Collection != materialize.CollectionPostV2 { + // Same deferral as the removal side: comment-level moderation state + // needs 17c-2's record. Taken and counted, never fallen through. + NativeCommentModerationDeferred.Add(1) + return skip(mapping.APID, + "announced restore of a native comment: moderation of native comments is not "+ + "implemented yet (needs the 17c-2 removal record), taking no action") + } + if err := h.tombstones.Remove(ctx, mapping.APID, scope); err != nil { + return fmt.Errorf("ingest: clear tombstone for %s: %w", mapping.APID, err) + } + if err := h.objects.Restore(ctx, mapping.APID); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("ingest: restore mapping for %s: %w", mapping.APID, err) + } + h.logger.Info("community lifted its removal of a native post; re-accepting", + "ap_id", mapping.APID, "at_uri", mapping.ATURI, "community", announcer.APGroupID, + "activity", undo.ID) + return h.mat.RestorePost(ctx, mapping) } // retractDeleteMarker is Undo{Delete} for an id with no mapping: the delete diff --git a/internal/ingest/echo_delete_test.go b/internal/ingest/echo_delete_test.go --- a/internal/ingest/echo_delete_test.go +++ b/internal/ingest/echo_delete_test.go @@ -66,7 +66,7 @@ // have written. mappingFields decides how much of the mapping is populated: // today's enqueuer leaves community_did and author_did empty, and 17c fills // community_did in — the difference between an accidental drop and a // destructive one. -func setupNativeDeleteEcho(t *testing.T, h *harness, communityDID, authorDID string) deleteEchoWorld { +func setupNativeDeleteEcho(t *testing.T, h *harness, authorDID string) deleteEchoWorld { t.Helper() ctx := context.Background() group := h.subscribeTechnology() @@ -112,6 +112,7 @@ Op: "create", ATURI: mdPostATURI, ID: consume.ActivityID(mdUserOrigin, mdPostATURI, "create", 0), CommunityAPID: groupID, + CommunityDID: actualCommunityDID, Snapshot: mdSnapshot(t), }) // 2. The AUTHOR deletes their own post: engine.go enqueues PostIntent{delete}. @@ -120,6 +121,7 @@ Op: "delete", ATURI: mdPostATURI, ID: consume.ActivityID(mdUserOrigin, mdPostATURI, "delete", 1), CommunityAPID: groupID, + CommunityDID: actualCommunityDID, Snapshot: mdSnapshot(t), } enqueueAs(t, h.db, enqueuer, mdAuthorDID, deleteIntent) @@ -141,7 +143,11 @@ // (The tripwire that stood here — "today's enqueuer records no community" — // fired in 17c-1 and has been removed. Its whole purpose was to fail on the // day M1b stopped simulating: community_did is populated now, so this // scenario is the real world rather than a construction of it.) - mapping.CommunityDID = communityDID + // community_did is NOT written here: it rides the intent and the enqueuer + // copies it, so asserting it is what pins that path. author_did has no such + // carrier yet, so it stays a fixture knob. + require.Equal(t, actualCommunityDID, mapping.CommunityDID, + "the enqueuer must bind the mapping to the community it federated into") mapping.AuthorDID = authorDID _, err = h.objects.PutMapping(ctx, *mapping) require.NoError(t, err, "rewrite the mapping the way this scenario's world has it") @@ -194,28 +200,19 @@ // gone: if the classifier does not catch the echo there, the bridge fabricates // a moderation record against an author who moderated nobody. func TestAnnouncedDeleteEchoNeverWritesAModeratorRemoval(t *testing.T) { cases := []struct { - name string - communityDID func(actual string) string - authorDID string - why string + name string + authorDID string + why string }{ { - name: "M1a mapping as today's enqueuer writes it", - communityDID: func(string) string { return "" }, - why: "today the echo dies on an accident — the community binding fails — and " + - "the drop must become the classifier's decision instead", - }, - { - name: "M1b mapping carrying its community DID, as 17c will leave it", - communityDID: func(actual string) string { return actual }, + name: "M1b mapping as the enqueuer now leaves it (community bound)", why: "with community_did populated the authorization PASSES, the summary-less " + "Delete is not provably the author's, and RemovePost writes a " + "community-signed removal against a self-delete", }, { - name: "M1b mapping carrying community AND author DIDs", - communityDID: func(actual string) string { return actual }, - authorDID: mdAuthorDID, + name: "M1b mapping carrying community AND author DIDs", + authorDID: mdAuthorDID, why: "author_did does not save it: deleteIsByAuthor resolves the author through " + "bridged_actors, and a NATIVE persona has no row there — the answer is " + "false however complete the mapping is", @@ -226,8 +223,7 @@ for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { h := newHarness(t) ctx := context.Background() - actualCommunityDID := testDIDFor("technology", "lemmy.world") - world := setupNativeDeleteEcho(t, h, tc.communityDID(actualCommunityDID), tc.authorDID) + world := setupNativeDeleteEcho(t, h, tc.authorDID) acceptanceBefore, acceptanceCIDBefore, err := h.manager.GetRecord(ctx, world.communityDID, materialize.CollectionAcceptance, world.digestRKey) diff --git a/internal/ingest/inbox.go b/internal/ingest/inbox.go --- a/internal/ingest/inbox.go +++ b/internal/ingest/inbox.go @@ -298,22 +298,35 @@ http.Error(w, "activity must carry id and type", http.StatusBadRequest) return } - // Bind the activity's claimed actor to the verified signer. Exact - // equality is the common case (Lemmy signs as the acting actor); same - // authority tolerates instance-actor signing (Mastodon secure-mode - // relays) without letting host A speak for host B. The QUEUED actor id - // is the activity's actor — downstream authorization (followed - // community, delete authority) keys off it. + // THE QUEUED ACTOR IS THE VERIFIED SIGNER, NEVER THE ACTIVITY'S CLAIM. + // + // Downstream authorization keys off this id, and several decisions turn on + // WHICH IDENTITY it is rather than which host it belongs to: the announcing + // community that may moderate its own content, and handleAccept/handleReject + // (which set communityID = signer and then compare communityID against + // signer — a comparison that cannot fail once the claim is trusted). Queuing + // the claim let any account on a host speak AS any other actor on that host: + // an ordinary user could remove a native post from a community it has + // nothing to do with, drive a pending follow to accepted, or unsubscribe us + // from a community outright. + // + // The previous tolerance existed for instance-actor signing (Mastodon + // secure-mode), but that applies to signed FETCHES, not delivery POSTs — + // Lemmy signs as the acting actor, and no fixture or live path we receive + // has an outer actor differing from its signer. So there is nothing to + // tolerate, and the identity has to be the unforgeable one. + // + // A CROSS-AUTHORITY claim is still refused outright rather than silently + // ignored: a delivery whose body claims another host's actor is malformed or + // hostile whichever id we end up keying on, and the sender should learn that + // at the door. + if claimed := refID(activity.Actor); claimed != "" && !ap.SameAuthority(claimed, actorID) { + ib.logger.Warn("inbox delivery rejected: actor/signer authority mismatch", + "activity_actor", claimed, "signer", actorID) + http.Error(w, "activity actor does not match signature", http.StatusForbidden) + return + } boundActor := actorID - if claimed := refID(activity.Actor); claimed != "" { - if !ap.SameAuthority(claimed, actorID) { - ib.logger.Warn("inbox delivery rejected: actor/signer authority mismatch", - "activity_actor", claimed, "signer", actorID) - http.Error(w, "activity actor does not match signature", http.StatusForbidden) - return - } - boundActor = claimed - } isNew, err := ib.events.Enqueue(r.Context(), store.InboxEvent{ ActivityID: activity.ID, diff --git a/internal/ingest/inbox_laundering_test.go b/internal/ingest/inbox_laundering_test.go new file mode 100644 --- /dev/null +++ b/internal/ingest/inbox_laundering_test.go @@ -0,0 +1,319 @@ +package ingest + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "tidepool/internal/errors" + "tidepool/internal/materialize" + "tidepool/internal/store" +) + +// ACTOR LAUNDERING AT THE INBOX. +// +// The inbox verifies ONE actor's signature and then lets the activity's `actor` +// field name a DIFFERENT actor on the same host, queueing that CLAIM as the +// event's bound ActorID. Downstream, handleAnnounce resolves the announcing +// community from that id — so the attacker chooses which community is treated +// as verified. +// +// That was defensible while every downstream decision was AUTHORITY-based +// ("lemmy.world may speak for lemmy.world's users"). Decision 18's rule is +// IDENTITY-based: the signer must BE the community that owns the target. The +// binding silently downgrades identity to authority, and 17c-1 is what made the +// difference reachable — before it, moderation of native content was refused for +// unrelated reasons. +// +// WHY THE EXISTING CROSS-COMMUNITY TEST CANNOT SEE THIS: it has community B sign +// AS ITSELF and be refused by the membership rule. The attack is a third party +// CLAIMING TO BE community A, which passes membership because the claim IS A. +// The laundering happens one layer earlier than the check we pinned, so a +// fixture that signs honestly cannot express it. +// +// Every test here asserts STATE, not HTTP status: refusing at the inbox (403) +// and queueing-then-refusing at the handler are both legitimate fixes, and +// pinning the status would pick one for GREEN. +const ( + // An ordinary user account on the SAME instance as the community. Nothing + // about this actor is privileged; that is the point. + lnAttacker = "https://lemmy.world/u/attacker" +) + +// laundered builds an Announce whose `actor` field claims to be claimedActor. +// The caller signs it with somebody else's key. +func laundered(activityID, claimedActor string, inner map[string]any) map[string]any { + return map[string]any{ + "@context": "https://www.w3.org/ns/activitystreams", + "id": activityID, + "type": "Announce", + "actor": claimedActor, + "audience": claimedActor, + "cc": []any{claimedActor + "/followers"}, + "object": inner, + } +} + +// innerModDelete is the live Lemmy mod-removal shape: a Delete carrying a +// summary, attributed to a moderator. +func innerModDelete(activityID, targetID, community, summary string) map[string]any { + return map[string]any{ + "id": activityID + "/delete", + "type": "Delete", + "actor": modActorID, + "object": targetID, + "summary": summary, + "audience": community, + "cc": []any{community}, + } +} + +// TestForgedAnnouncerCannotRemoveNativeContent is L1: the attack itself. +// +// A user account signs the delivery; the body claims to be the community. Both +// are on lemmy.world, so the same-authority tolerance admits it and the queued +// ActorID becomes the community the ATTACKER named. +// +// If this passes to the handler, any account on any instance can withdraw native +// content from any community co-hosted with it — the moderation surface 17c-1 +// opened, driven by whoever asks. +func TestForgedAnnouncerCannotRemoveNativeContent(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + world := newModerationWorld(t, h) + attacker := h.newRemoteActor(lnAttacker, person(lnAttacker, "attacker", nil)) + + activitiesBefore := rowCount(t, h.db, "outbound_activities") + + _ = h.deliver(attacker, laundered( + "https://lemmy.world/activities/announce/delete/ln-forged", + groupID, // the CLAIM: community A + innerModDelete("https://lemmy.world/activities/announce/delete/ln-forged", + mtPostAPID, groupID, "removed by someone who is not a moderator"))) + h.drain() + + _, _, err := h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionRemoval, world.digestRKey) + assert.True(t, errors.IsNotFound(err), + "a delivery SIGNED BY a user account must never act as the community it names: the "+ + "signature is the only evidence of identity we have, and everything decision 18 "+ + "rules on is downstream of it (err=%v)", err) + + _, _, err = h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionAcceptance, world.digestRKey) + assert.NoError(t, err, + "and the post stays accepted: the community said it belongs there and no one has "+ + "said otherwise") + + assert.Equal(t, activitiesBefore, rowCount(t, h.db, "outbound_activities"), + "nothing may go outbound off a forged delivery") +} + +// TestForgedAnnouncerCannotRestoreRemovedContent is L2, the mirror. +// +// Restore is the same authority in the other direction: a standing moderator +// removal must not be liftable by anyone who can spell the community's id. If +// only the removal path is fixed, an attacker cannot remove a post — but can +// reinstate every post the moderators removed, which is the same power. +func TestForgedAnnouncerCannotRestoreRemovedContent(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + world := newModerationWorld(t, h) + attacker := h.newRemoteActor(lnAttacker, person(lnAttacker, "attacker", nil)) + + // The community's own moderator removes the post, honestly signed. + reason := "removed by an actual moderator" + h.announceDeleteWithSummary(world.groupA, + "https://lemmy.world/activities/announce/delete/ln-real", mtPostAPID, &reason) + standing, _, err := h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionRemoval, world.digestRKey) + require.NoError(t, err, "precondition: a real removal stands") + require.Equal(t, reason, standing["reason"]) + + // The attacker tries to lift it, claiming to be the community. + inner := innerModDelete("https://lemmy.world/activities/announce/delete/ln-real", + mtPostAPID, groupID, reason) + _ = h.deliver(attacker, laundered( + "https://lemmy.world/activities/announce/undo/ln-forged-restore", + groupID, + map[string]any{ + "id": "https://lemmy.world/activities/announce/undo/ln-forged-restore/undo", + "type": "Undo", + "actor": modActorID, + "audience": groupID, + "cc": []any{groupID}, + "object": inner, + })) + h.drain() + + stillRemoved, _, err := h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionRemoval, world.digestRKey) + assert.NoError(t, err, + "the moderators' removal must survive a forged restore: reinstating content they "+ + "removed is the same power as removing content they kept") + if err == nil { + assert.Equal(t, reason, stillRemoved["reason"], "unchanged, with their reason") + } + + _, _, err = h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionAcceptance, world.digestRKey) + assert.True(t, errors.IsNotFound(err), + "and no acceptance may be written back: that is what would make the post visible "+ + "again in Coves (err=%v)", err) + + // THE REFUSAL MUST BE A DECISION. Today the forged restore is authorized and + // reaches re-materialization; it fails only because that writes into the + // native AUTHOR's repo, which the bridge does not host — an accident of + // where native posts live, not a judgement about who sent this. The event is + // then left retrying against a forgery, which is what an unrefused attack + // looks like from the queue's side. + event, err := h.events.GetEvent(ctx, + "https://lemmy.world/activities/announce/undo/ln-forged-restore") + require.NoError(t, err) + assert.NotNil(t, event.ProcessedAt, + "a forged delivery must be DECIDED — skipped, once. Left retrying, it means the "+ + "identity was accepted and only the write failed: the same forgery against a "+ + "target whose repo we DO host would land (last error: %s)", event.Error) + assert.Nil(t, event.FailedAt, "and it must not poison either: nothing here is retryable") +} + +// TestSiblingCommunityCannotImpersonateTheOwningCommunity is L4. +// +// This is the shape closest to the cross-community test that already passes, and +// the difference is the whole point: there, B signs as B and the MEMBERSHIP rule +// refuses it (A's post is not B's). Here B signs as B but CLAIMS to be A, so the +// membership rule compares A against A and passes. +// +// A fix that only tightens membership therefore cannot satisfy this test — the +// identity has to stop being forgeable. +func TestSiblingCommunityCannotImpersonateTheOwningCommunity(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + world := newModerationWorld(t, h) + + _ = h.deliver(world.groupB, laundered( + "https://lemmy.world/activities/announce/delete/ln-sibling", + groupID, // community B signs, but claims to be community A + innerModDelete("https://lemmy.world/activities/announce/delete/ln-sibling", + mtPostAPID, groupID, "sibling community claiming to be the owner"))) + h.drain() + + _, _, err := h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionRemoval, world.digestRKey) + assert.True(t, errors.IsNotFound(err), + "community B may not become community A by saying so: membership passes here (the "+ + "claim IS the owning community), so only the signature can refuse it (err=%v)", err) + + _, _, err = h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionAcceptance, world.digestRKey) + assert.NoError(t, err, "A's acceptance is untouched") +} + +// TestHonestlySignedModerationStillWorks is L3, the control. +// +// The tolerance being exploited exists to admit deliveries whose signer is not +// byte-identical to the activity's actor. Whatever GREEN does to it, the +// ordinary path must keep working: the community signs its own Announce and +// moderates its own content. A fix that refuses this refuses all moderation. +func TestHonestlySignedModerationStillWorks(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + world := newModerationWorld(t, h) + + reason := "genuinely off topic" + h.announceDeleteWithSummary(world.groupA, + "https://lemmy.world/activities/announce/delete/ln-honest", mtPostAPID, &reason) + + removal, _, err := h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionRemoval, world.digestRKey) + require.NoError(t, err, + "the community moderating its OWN content is the behaviour 17c-1 exists to deliver; "+ + "a laundering fix that also refuses this has removed the feature") + assert.Equal(t, reason, removal["reason"]) + assert.Equal(t, "moderator-discretion", removal["code"]) +} + +// TestCrossAuthorityClaimIsStillRejected pins the half of the binding that is +// unambiguously right, so a fix cannot regress it while rewriting the rest: a +// claimed actor on a DIFFERENT host than the signer is refused outright. +// +// This is the case the current comment is really about — "without letting host A +// speak for host B" — and it must survive whatever replaces the same-authority +// tolerance. +func TestCrossAuthorityClaimIsStillRejected(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + world := newModerationWorld(t, h) + foreign := "https://evil.example/u/attacker" + attacker := h.newRemoteActor(foreign, person(foreign, "attacker", nil)) + + _ = h.deliver(attacker, laundered( + "https://evil.example/activities/announce/delete/ln-cross-authority", + groupID, + innerModDelete("https://evil.example/activities/announce/delete/ln-cross-authority", + mtPostAPID, groupID, "from another host entirely"))) + h.drain() + + _, _, err := h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionRemoval, world.digestRKey) + assert.True(t, errors.IsNotFound(err), + "a cross-authority claim is refused at the door and must stay refused (err=%v)", err) + _, _, err = h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionAcceptance, world.digestRKey) + assert.NoError(t, err) +} + +// TestForgedAcceptCannotSubscribeUsToACommunity is the second reachable +// escalation, reported alongside the moderation one. +// +// handleAccept resolves the community from the same bound id and then checks +// `communityID != signer` — which, under laundering, compares the claim against +// itself and can never fail. So any account on the instance can drive a pending +// follow to ACCEPTED and trigger a backfill of that community's whole outbox. +// +// The follow state machine is not moderation, but it is the same forged +// identity, and it is the path that pulls content INTO the bridge. +func TestForgedAcceptCannotSubscribeUsToACommunity(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + _ = newModerationWorld(t, h) + attacker := h.newRemoteActor(lnAttacker, person(lnAttacker, "attacker", nil)) + + // A third community on the same instance, subscribed but NOT yet accepted. + const pendingAPID = "https://lemmy.world/c/pending" + _, err := h.communities.UpsertCommunity(ctx, store.Community{ + APGroupID: pendingAPID, + DID: testDIDFor("pending", "lemmy.world"), + PreferredUsername: "pending", + Instance: "lemmy.world", + }) + require.NoError(t, err) + require.NoError(t, h.communities.SetFollowState(ctx, pendingAPID, store.FollowStatePending)) + backfillsBefore := h.backfills.count() + + _ = h.deliver(attacker, map[string]any{ + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://lemmy.world/activities/accept/ln-forged", + "type": "Accept", + "actor": pendingAPID, // the CLAIM + "object": map[string]any{ + "id": "https://" + bridgeHost + "/activities/follow/whatever", + "type": "Follow", + "actor": h.service.ID, + "object": pendingAPID, + }, + }) + h.drain() + + community, err := h.communities.GetByAPGroupID(ctx, pendingAPID) + require.NoError(t, err) + assert.Equal(t, store.FollowStatePending, community.FollowState, + "only the community itself can accept our Follow: followCommunity's check compares "+ + "the claimed community against the bound id, so under laundering it compares a "+ + "value with itself and cannot fail") + assert.Equal(t, backfillsBefore, h.backfills.count(), + "and no backfill may be triggered by a stranger: it is an outbound crawl of a whole "+ + "community's history, started by whoever asks") +} diff --git a/internal/ingest/ingest_test.go b/internal/ingest/ingest_test.go --- a/internal/ingest/ingest_test.go +++ b/internal/ingest/ingest_test.go @@ -317,12 +317,18 @@ }) h.minter = &fakeMinter{custodian: custodian} h.mat, err = materialize.New(materialize.Options{ - Fetcher: h.client, - Objects: objects, - Actors: actors, - Communities: communities, - Repos: manager, - Minter: h.minter, + Fetcher: h.client, + Objects: objects, + Actors: actors, + Communities: communities, + Repos: manager, + Minter: h.minter, + // Wired because PRODUCTION wires it: without it restorePin's entire + // bridge-origin branch takes the outbound == nil warn-and-refuse path, + // so the origin dispatch, the tombstone refusal and the empty-CID + // refusal are all dead code in every ingest test — passing by never + // running. + OutboundObjects: store.NewOutboundObjects(database), ServiceDID: testServiceDID, StrictValidation: true, }) diff --git a/internal/ingest/moderation_terminal_test.go b/internal/ingest/moderation_terminal_test.go --- a/internal/ingest/moderation_terminal_test.go +++ b/internal/ingest/moderation_terminal_test.go @@ -115,6 +115,12 @@ Custodian: h.custodian, UserOrigin: mtUserOrigin, }) require.NoError(t, err) + // The user origin SERVES the post for real. The restore path re-fetches its + // target from that target's own authority and re-materializes what comes + // back, so without a live origin a restore is refused by the fetch failing — + // and a test asserting "the restore was refused" would be passing on the + // wrong reason entirely. + h.mux.Handle("/ap/", userOrigin) enqueuer, err := outbound.NewEnqueuer(outbound.EnqueuerOptions{ DB: h.db, Translator: outbound.NewTranslator(mtUserOrigin), @@ -153,18 +159,18 @@ digest := testDigestRKey(mtPostATURI) _, _, err = h.manager.GetRecord(ctx, communityADID, materialize.CollectionAcceptance, digest) require.NoError(t, err, "precondition: the post is accepted into community A") - // --- The 17c PREREQUISITE, constructed the way 17c will leave the world. - // Without community_did on the mapping, CommunityDIDOf returns "" and - // every announced moderation action is refused before it is evaluated — - // which is the accident that has been standing in for authorization. - // GREEN carries this column through the intent; the fixture states the - // end state so these behaviours can be pinned against it. + // --- The 17c PREREQUISITE, asserted rather than constructed. Every + // moderation behaviour below is downstream of this column, so a fixture + // that WROTE it would let an implementation that never populates it pass + // the whole suite — the intent field, the enqueuer's copy, and the + // backfill would all be unpinned by the tests that depend on them most. mapping, err := h.objects.GetByAPID(ctx, mtPostAPID) require.NoError(t, err, "the enqueuer maps the federated post") require.Equal(t, store.OriginBridge, mapping.Origin) - mapping.CommunityDID = communityADID - _, err = h.objects.PutMapping(ctx, *mapping) - require.NoError(t, err) + require.Equal(t, communityADID, mapping.CommunityDID, + "the enqueuer must bind the mapping to the community it federated into: "+ + "CommunityDIDOf reads this column, and an empty one refuses every announced "+ + "moderation action before it is evaluated") return moderationWorld{ groupA: groupA, groupB: groupB, @@ -231,6 +237,15 @@ h := newHarness(t) ctx := context.Background() world := newModerationWorld(t, h) + // Snapshotted BEFORE the removal, not after: an enqueue caused by the + // REMOVAL ITSELF would otherwise be folded into the baseline and invisible. + // Boomerang suppression is structural today (materialize.RemovePost uses + // ApplyOps, which takes no side effect and so cannot enqueue), but nothing + // stops a refactor to ApplyOpsTx, and the failure would be a Delete{Page} + // sent back at the community that just removed the post. + activitiesAtStart := rowCount(t, h.db, "outbound_activities") + deliveriesAtStart := rowCount(t, h.db, "outbound_deliveries") + // --- A moderator of community A removes the post: Delete WITH summary, // announced by the community that owns it. reason := "off topic for this community" @@ -249,6 +264,11 @@ require.True(t, errors.IsNotFound(err), "precondition: the acceptance was withdrawn (err=%v)", err) activitiesBefore := rowCount(t, h.db, "outbound_activities") deliveriesBefore := rowCount(t, h.db, "outbound_deliveries") + assert.Equal(t, activitiesAtStart, activitiesBefore, + "the REMOVAL itself must enqueue nothing: an inbound moderation action is the "+ + "community telling US what it did, and echoing it back is a Delete{Page} aimed "+ + "at the moderators who sent it") + assert.Equal(t, deliveriesAtStart, deliveriesBefore, "...and no delivery") // --- WHEN: the author edits their post. An ordinary commit, the kind that // happens minutes later when someone fixes a typo. @@ -340,3 +360,89 @@ assert.NoError(t, err, "a refused cross-community removal must not leave the post in a state where its own "+ "community's edits stop working") } + +// TestAnnouncedRestoreOfANativePostLiftsTheRemovalCleanly is RESTORE-1. +// +// Populating community_did made announced Undo{Delete} reachable for +// BRIDGE-ORIGIN content for the first time, and that path has no origin guard. +// handleUndoDelete re-fetches the target from its own authority — which for a +// native post is OUR OWN /ap/object/… id — and hands the result to +// mat.HandleUpdate DIRECTLY, bypassing materializeContent's bridge-origin echo +// guard, the only place that says "this is ours". +// +// What follows is a chain of consequences, each worse than the last: +// +// HandleUpdate → MaterializePost → EnsureActor(our own persona's actor id) +// → MINTS a PLC DID and a bridged_actors row for a native Coves user; +// then commitRecord targets the AUTHOR's repo, which the bridge does not host, +// so signing fails and HandleUpdate errors; +// then the error path compensates by SOFT-DELETING our own bridge-origin +// mapping and recording a tombstone for our own AP id — after which +// moderateAnnouncedDelete declines forever on mapping.IsDeleted(). +// +// That last step is the one that does not wash out: the post becomes +// permanently unmoderatable, by the community's own legitimate restore. +// +// There is nothing to re-materialize here. The record lives in the author's +// repo and the removal never touched it; a restore of a native post is the +// acceptance coming back, nothing more. +func TestAnnouncedRestoreOfANativePostLiftsTheRemovalCleanly(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + world := newModerationWorld(t, h) + + reason := "removed, then reconsidered" + h.announceDeleteWithSummary(world.groupA, + "https://lemmy.world/activities/announce/delete/mt-restore", mtPostAPID, &reason) + _, _, err := h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionRemoval, world.digestRKey) + require.NoError(t, err, "precondition: the removal stands") + + bridgedBefore := rowCount(t, h.db, "bridged_actors") + + // The community lifts its own removal, honestly signed. + h.announceUndoDelete(world.groupA, + "https://lemmy.world/activities/announce/undo/mt-restore", + "https://lemmy.world/activities/announce/delete/mt-restore/delete", + mtPostAPID, &reason) + + // --- The restore lands: the acceptance is back. + _, _, err = h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionAcceptance, world.digestRKey) + assert.NoError(t, err, + "the community's own Undo{Delete} must re-accept the post: a removal the moderators "+ + "lifted that stays standing is moderation nobody can undo") + _, _, err = h.manager.GetRecord(ctx, + world.communityADID, materialize.CollectionRemoval, world.digestRKey) + assert.True(t, errors.IsNotFound(err), + "and the removal is gone with it — acceptance and removal share one rkey and one "+ + "commit precisely so neither outlives the other (err=%v)", err) + + // --- And nothing of ours was mistaken for remote content on the way. + assert.Equal(t, bridgedBefore, rowCount(t, h.db, "bridged_actors"), + "NO bridged actor may be minted: EnsureActor runs on the re-materialization path, "+ + "and a native Coves user acquiring a second, bridge-minted fediverse identity is "+ + "the mint oracle this loop has closed twice already") + + mapping, err := h.objects.GetByAPID(ctx, mtPostAPID) + require.NoError(t, err) + assert.False(t, mapping.IsDeleted(), + "our own mapping must not be soft-deleted: the compensation path does that when the "+ + "re-materialization fails, and moderateAnnouncedDelete then declines forever on "+ + "IsDeleted() — the post becomes permanently unmoderatable, by a legitimate restore") + assert.Equal(t, store.OriginBridge, mapping.Origin, + "and it must still be ours: re-materializing rewrites the row as fediverse-origin, "+ + "which silently disables the echo guard for this post") + + tombstoned, err := h.tombstones.ExistsFor(ctx, mtPostAPID, groupID) + require.NoError(t, err) + assert.False(t, tombstoned, + "nor may a tombstone be recorded against our own AP id: it suppresses this post's "+ + "own later Creates and drops every Lemmy reply beneath it") + + event, err := h.events.GetEvent(ctx, "https://lemmy.world/activities/announce/undo/mt-restore") + require.NoError(t, err) + assert.NotNil(t, event.ProcessedAt, + "and the restore is DECIDED, not left retrying: %s", event.Error) + assert.Nil(t, event.FailedAt, "nor poisoned") +} diff --git a/internal/materialize/acceptance.go b/internal/materialize/acceptance.go --- a/internal/materialize/acceptance.go +++ b/internal/materialize/acceptance.go @@ -112,6 +112,11 @@ // removing a post says where the post may appear, not whether it exists; // deleting the author's record would let one community destroy content for // every other, and tombstoning the mapping would block the post's later edits // and votes from ever materializing again. +// removalCodeModeratorDiscretion is the removal lexicon's catch-all: Lemmy +// sends no machine-readable code, so anything narrower would be the bridge +// asserting a reason the moderator never gave. +const removalCodeModeratorDiscretion = "moderator-discretion" + func (m *Materializer) RemovePost(ctx context.Context, mapping *store.APObjectMapping, reason string) error { communityDID, postURI, rkey, err := m.moderationTarget(ctx, mapping) if err != nil { @@ -145,7 +150,7 @@ "subject": strongRef(postURI, pinned), // Lemmy sends no machine-readable code, so the open knownValues set's // catch-all applies. Inventing a narrower code (spam, rule-violation) // would be the bridge asserting a reason the moderator never gave. - "code": "moderator-discretion", + "code": removalCodeModeratorDiscretion, "createdAt": recordDatetime(m.moderationStamp(ctx, communityDID, CollectionRemoval, rkey)), } // Omitted rather than written blank: Lemmy spells "no reason given" as an @@ -166,9 +171,20 @@ return fmt.Errorf("materialize: remove %s from %s: %w", postURI, communityDID, err) } m.logger.Info("post removed from community by moderator", "community_did", communityDID, "post", postURI, "ap_id", mapping.APID) + m.recordModeration(ctx, mapping, func() error { + return m.ledger.RecordRemoval(ctx, communityDID, postURI, mapping.DID, removalCodeModeratorDiscretion) + }) return nil } +// CHILDREN ARE NOT TOUCHED: replies to and votes on a removed post keep +// resolving it as a live subject and keep federating. That matches Lemmy, where +// a removed post's comment thread survives, and it is deliberate rather than an +// oversight — but it is NOT yet enforceable state on our side: comment-level +// removal needs the object_moderation table 17c-2 introduces. Until then a +// community that removes a post and then wants its thread stopped has no +// mechanism here. +// // RestorePost undoes a moderator removal: the removal is deleted and a fresh // acceptance written IN ONE COMMIT, for the same reason the removal was // atomic. It is a no-op when no removal stands, so a restore that arrives @@ -219,7 +235,30 @@ return fmt.Errorf("materialize: restore %s into %s: %w", postURI, communityDID, err) } m.logger.Info("post restored to community by moderator", "community_did", communityDID, "post", postURI, "ap_id", mapping.APID) + m.recordModeration(ctx, mapping, func() error { + return m.ledger.RecordRestore(ctx, communityDID, postURI, mapping.DID, currentCID) + }) return nil +} + +// recordModeration mirrors a committed moderation transition into the +// admissions ledger, for NATIVE posts only: the ledger is the acceptance +// engine's record of what it decided about a native author's post, and a +// fediverse-origin post has no admission to update. +// +// It runs AFTER the community-repo commit and its failure is LOGGED, never +// returned. The repo records are the source of truth for removal state; the +// ledger is the operator surface over them. Failing the whole activity — and +// redelivering a moderation action that already committed — to fix a reporting +// row would trade a correct decision for a repeated one. +func (m *Materializer) recordModeration(ctx context.Context, mapping *store.APObjectMapping, write func() error) { + if m.ledger == nil || mapping.Origin != store.OriginBridge { + return + } + if err := write(); err != nil { + m.logger.Warn("moderation applied but the admissions ledger was not updated", + "at_uri", mapping.ATURI, "ap_id", mapping.APID, "error", err) + } } // restorePin is the CID a fresh acceptance pins, dispatched on ORIGIN because diff --git a/internal/materialize/materializer.go b/internal/materialize/materializer.go --- a/internal/materialize/materializer.go +++ b/internal/materialize/materializer.go @@ -138,6 +138,22 @@ type VoteScrubber interface { ScrubVoter(ctx context.Context, voterAPID string) error } +// ModerationLedger records a community moderation decision against a NATIVE +// post in the admissions ledger — the operator surface that answers "why is +// this post not in the community?". +// +// It is an INTERFACE rather than an *accept.Admissions so this package keeps no +// dependency on the acceptance engine (which already depends on the stores this +// one writes through); main adapts the concrete type. +type ModerationLedger interface { + // RecordRemoval marks the post removed by its community, with the removal + // record's own code. authorDID is the repo the post lives in. + RecordRemoval(ctx context.Context, communityDID, postURI, authorDID, code string) error + // RecordRestore marks the post accepted again, pinning the CID the fresh + // acceptance was written against. + RecordRestore(ctx context.Context, communityDID, postURI, authorDID, cid string) error +} + // Options configures New. Fetcher, Objects, Actors, Communities, Repos, // Minter, and ServiceDID are required. type Options struct { @@ -150,6 +166,15 @@ Minter ActorMinter // Votes scrubs a deleted actor's vote_events rows alongside the record // scrub (optional; nil skips it). Votes VoteScrubber + // Ledger records inbound moderation decisions against NATIVE posts in the + // admissions ledger, so a moderator's removal is visible there when the + // MODERATOR acts rather than only if the author later edits (which is the + // only thing that writes a row otherwise). It also frees the author's + // per-community rate quota, which counts accepted rows. + // + // OPTIONAL: nil skips the ledger write and changes nothing else — the + // community repo records remain the source of truth for removal state. + Ledger ModerationLedger // OutboundObjects is the bridge's own outbound state for NATIVE records. // RestorePost needs it: a native post lives in the AUTHOR's repo, which // this bridge does not host, so its CID cannot be read back through Repos. @@ -180,6 +205,7 @@ type Materializer struct { fetcher Fetcher objects store.APObjects outbound store.OutboundObjects + ledger ModerationLedger actors store.BridgedActors communities store.Communities repos *repo.Manager @@ -240,6 +266,7 @@ m := &Materializer{ fetcher: opts.Fetcher, objects: opts.Objects, outbound: opts.OutboundObjects, + ledger: opts.Ledger, actors: opts.Actors, communities: opts.Communities, repos: opts.Repos, diff --git a/internal/outbound/enqueuer.go b/internal/outbound/enqueuer.go --- a/internal/outbound/enqueuer.go +++ b/internal/outbound/enqueuer.go @@ -201,6 +201,12 @@ APType: apType, OriginInstance: e.originHost, Origin: store.OriginBridge, DID: parts[0], + // The AUTHOR is the at-uri's repo: an author-owned postv2 or comment + // lives in their own repo, so DID and AuthorDID are the same DID here. + // Recorded rather than left empty because deleteIsByAuthor reads this + // column to tell a self-delete from a moderator removal, and an empty + // one answers "not provably the author" for every native record. + AuthorDID: parts[0], // The COMMUNITY this object was federated into. Without it // CommunityDIDOf answers "" for a bridge-origin mapping — an // author-owned postv2 lives in the AUTHOR's repo, so the community diff --git a/internal/store/ap_objects.go b/internal/store/ap_objects.go --- a/internal/store/ap_objects.go +++ b/internal/store/ap_objects.go @@ -55,7 +55,12 @@ ON CONFLICT (ap_id) DO UPDATE SET ap_type = EXCLUDED.ap_type, origin = EXCLUDED.origin, did = EXCLUDED.did, - author_did = EXCLUDED.author_did, + -- author_did gets the same COALESCE treatment, and for a sharper + -- reason than tidiness: deleteIsByAuthor decides SELF-DELETE vs + -- MODERATOR REMOVAL from this column, and a re-put that omitted it + -- would silently turn every later author delete into "not provably + -- the author" — the branch that writes a moderation record. + author_did = COALESCE(EXCLUDED.author_did, ap_objects.author_did), -- COALESCE, never a bare overwrite: community_did is the binding -- that authorizes announced moderation of this object, and a -- re-put that simply omits it (a re-materialization, a legacy