feat(feed): mentions + stranger involvement, expand replies, reply backfill, bell reconciliation (#1060 r2) - #1064
Conversation
…ckfill, bell reconciliation (#1060 round 2) Fredrik's follow-ups from using the timeline against Mastodon publishers: - Legacy replies still showed as top-level cards (ingested before in_reply_to_uri existed): timeline reads now lazily backfill a few unchecked entries per read (AP re-fetch, one attempt, stamped via new reply_checked_at; fresh ingests stamp immediately) - timeline_show_replies toggle now ALSO lives on the Feed page next to the Home timeline (saving invalidates the timeline query) - Involvement now includes Mentions and works for strangers: the inbox admits a non-followee Note when it replies to one of the recipient's own (existing) posts or Mentions them (author snapshot from the signature-verified sender); new mentions_me column/field keeps such posts visible and notifying whatever the reply setting says - Expand replies: GET /feed/timeline/:id/replies + MCP get_timeline_replies fetch a live bounded snapshot of the remote replies collection (≤20 replies/≤15 SSRF-guarded fetches/12s, sanitised, authors memoised); web cards get a Show replies button - Android: notifier notifies involvement regardless of the per-follow bell set; AutoEnablePostNotifications reconciles active bells with the device on launch (requests permission + starts the poller unless an explicit off was ever chosen) — bells can no longer silently lie Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwoP1SqJhHgHiEEoEQtUjT
fiddur
left a comment
There was a problem hiding this comment.
🛑 Changes required
Solid round 2 overall: the stranger-involvement admission is properly narrow (own-post reply verified against feed_post, or an exact-href Mention, plus the activity.actorId/getActor identity check and the existing noteToTimelineInput host/attribution guards), the reply fetch is genuinely budgeted and SSRF-guarded, getTimelineEntryById is per-user-DB scoped, the route validates the UUID before touching the DB, and the Android involvement rule + docs line up with the backend.
Two things should change before merge (both inline):
-
The reply backfill destroys correct data.
developalready storesin_reply_to_uri(shipped in #1061), and this PR's migration leavesreply_checked_atNULL on all existing rows — including those correctly-stamped ones.backfillReplyLinksthen writesparseReplyInfo(null, …)(i.e.in_reply_to_uri = NULL) whenever the re-fetch fails, which is common: post deleted upstream (404), instance in authorized-fetch/secure mode (401 for our unsigned GET), host down, or an HTML body returned for the APAccept. Those replies then resurface as top-level cards and losein_reply_to_mine, so "replied to you" markers and the Android involvement notification disappear. The unit test currently asserts the clobbering behaviour, so it needs updating too. -
TimelineReply.urlis an unvalidated remote string rendered into anhref.remote-replies.tssanitises the reply HTML but passesobj.urlstraight through, andHomeTimeline.tsxrenders<a href={reply.url}>. Ajavascript:URL from a hostile origin executes in the app origin on click — and thanks to mention-based ingestion, any stranger can now get a card (and therefore a "Show replies" button pointing at their own server) into a user's timeline. An http(s)-only check next to the existingsanitizeRemoteHtmlcall fixes it for both the REST and MCP consumers.
Non-blocking — fold into a later PR, don't re-roll this one
federation.ts: the updatedingestFeedActivitydocblock is now orphaned — it sits directly above the newingestStrangerInvolvementdocblock, so two doc comments stack on the stranger helper andingestFeedActivityhas none. It also describes only the reply branch, not the Mention-only one.listReplyUncheckedEntries(WHERE reply_checked_at IS NULL ORDER BY received_at DESC) has no supporting index, and there is no index onreceived_atat all — the analogous retro-enrich query gotidx_timeline_entry_unenrichedfor exactly this reason (cheap no-work case on every timeline read).- MCP
get_timeline_repliesreturnsjsonResponse({ error, success: false })where the rest of the file useserrorResponse(...), and re-inlines the12_000budget the router keeps asREPLIES_TIMEOUT_MS. invalidateQueries({ queryKey: ['feed', 'timeline'] })prefix-matches['feed','timeline',<id>,'replies'], so toggling the setting refetches every expanded thread from its remote origin.partialis reportedtruefor a thread of exactly 20 replies even when the collection was exhausted; conversely an unreachable origin returnspartial: false+ no replies, which the web renders as "No replies found on the origin." rather than a fetch failure.resolveAuthordoesnew URL(actorUri)on a rawattributedTovalue; a non-URL string throws and (via the route's.catch) collapses the whole thread to empty.noteMentionsActorand the stranger-admission branch — the security-relevant part of this PR — have no unit tests, andorigin?: stringoningestNoteForRecipientsilently disables mention detection if a future caller omits it (all current callers pass it, so it could just be required).- Involvement now notifies even for an account whose bell the user explicitly turned off. Intended per the docs, just worth being deliberate about — it does override an explicit mute.
- The comment above
AutoEnablePostNotifications()inMainActivity.ktrestates the function's own KDoc.
…round 1) - Backstamp migration marks rows that already carry in_reply_to_uri (ingested between #1061 and now) as checked, and a backfill fetch that yields no usable AS2 object (404, authorized-fetch 401, HTML body, host down) only stamps reply_checked_at — never overwrites stored reply state (new markTimelineEntryReplyChecked) - TimelineReply.url is kept only when http(s): a hostile origin — reachable by any stranger via mention ingestion — could otherwise put a javascript: href in the web's reply list - resolveAuthor no longer throws on a non-URL attributedTo (it collapsed the whole thread to empty via the route's catch) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwoP1SqJhHgHiEEoEQtUjT
fiddur
left a comment
There was a problem hiding this comment.
🛑 Changes required
Round 2 fixes from the last round both landed cleanly: the backfill is now no-clobber (markTimelineEntryReplyChecked on a non-answer, plus the timeline_entry_reply_checked_backstamp migration for rows that already carry a reply link — and the unit test asserts the new behaviour), TimelineReply.url is http(s)-only, and resolveAuthor no longer throws on a non-URL attributedTo. The reply walk is genuinely budgeted and SSRF-guarded, getTimelineEntryById is per-user-DB scoped, the route validates the UUID before touching the DB, and the migration keys are non-table names so they re-run on every migrateSchema pass as intended.
Two things should still change before merge (both inline, both one-liners):
- A stranger can overwrite a followee's timeline entry.
noteToTimelineInput's authority check only enforces attribution when the Note declaresattributedTo; with it omitted, any sender may claim anyobject_urion their own host. That guard was written for accepted followees ("a malicious accepted followee could deliver a Note whose id collides with another followee's post"), and the stranger branch widens it from "people you chose to follow" to "anyone sharing a host with someone you follow". TimelineReply.published_atis an unvalidated remote string declared asISO8601DateTime, and the web feeds it straight toformatDistanceToNow, which throwsRangeError: Invalid time valueon an unparseable value. There is no error boundary inapps/web, so one malformedpublishedtakes the timeline down.
Non-blocking — fold into a later PR, don't re-roll this one
federation.ts: theingestFeedActivitydocblock is still orphaned aboveingestStrangerInvolvement(two stacked doc comments;ingestFeedActivityhas none), and the.on(Create, …)comment now claims "the only stranger Note admitted is a reply to one of the recipient's own posts" — the Mention branch is missing from both, so the comments understate what the code admits.origin?: stringoningestNoteForRecipientis now passed by every caller; making it required would remove the silent "mentions never detected" failure mode for a future caller.- Still open from round 1: no index behind
listReplyUncheckedEntries(WHERE reply_checked_at IS NULL ORDER BY received_at DESC, run on every timeline read); MCPget_timeline_repliesusesjsonResponse({ error, success: false })where the rest of the file useserrorResponse(...)and re-inlines12_000;partialistruefor an exhausted thread of exactly 20 andfalsefor an unreachable origin (which the web then shows as "No replies found on the origin");invalidateQueries({ queryKey: ['feed', 'timeline'] })prefix-matches the per-entry replies queries. - Android: when the permission request is denied,
setPostNotificationsEnabled(context, false)records an explicit "off", and the Account screen's warning only fires on toggle-on + permission-off — so a user with bells on who denies once gets neither notifications nor the warning, which is the silent-bell case this reconciliation exists to remove.
… timestamps (#1064 round 2) - Stranger involvement branch now requires the Note to explicitly attribute to the signing actor — without attributedTo, anyone on a followee's host could claim an existing entry's object_uri and overwrite it via the upsert (and the actor-scoped Delete would then never match) - TimelineReply.published_at kept only when it parses: the schema promises ISO-or-null and date-fns throws on Invalid Date, which would blank the page on a hostile published value Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwoP1SqJhHgHiEEoEQtUjT
fiddur
left a comment
There was a problem hiding this comment.
🛑 Changes required
Solid round overall. The stranger-admission tightening holds up under scrutiny: explicit attributedTo == signing actor, the existing-own-post lookup, and the note.id.host === senderHost guard inherited from noteToTimelineInput together close the object_uri-overwrite path I went looking for. The no-clobber backfill is right too (markChecked vs saveReplyInfo, plus the idempotent backstamp so any row that already carries in_reply_to_uri is never a candidate — which also means every candidate has a NULL link, so a null result can't clobber anything). The listTimelineEntries filter change is safe (mentions_me is NOT NULL DEFAULT false, no three-valued logic), /timeline/:id/replies is registered ahead of /:postId, the web PATCHes settings partially so the Feed toggle can't wipe other settings, and the ['userSettings'] key matches the Settings page.
Blocking: one authenticity hole in the new reply fetcher — see the inline comment on remote-replies.ts.
Non-blocking — fold into a later PR, don't re-roll this one
- Two doc comments now document the wrong symbol.
timeline-ingest.ts:84— thenoteToTimelineInput"Authority check" block is now attached toexport type TimelineAuthor.federation.ts:203-221— theingestFeedActivityblock ("Two admissible senders…") is stacked directly aboveingestStrangerInvolvement, which has its own block, so the text is duplicated andingestFeedActivity(line 262) is left undocumented. - No index for the backfill candidate query.
listReplyUncheckedEntriesisWHERE reply_checked_at IS NULL ORDER BY received_at DESC LIMIT 3and runs on every timeline read, but the only index is(published_at DESC, id DESC)— so it is a seq scan + top-N sort, worst exactly once the backlog drains. #996 addedidx_timeline_entry_unenrichedas a partial index for precisely this shape; a(received_at DESC) WHERE reply_checked_at IS NULLpartial index would match that precedent. fetchRemoteRepliesreturnspartial: falsewhen the post itself couldn't be fetched (budget.exhaustedis still false), so an unreachable or authorized-fetch origin renders as "No replies found on the origin." — indistinguishable from a genuinely empty thread. Separately,partialis true whenever exactlyMAX_REPLIESwere collected, even when the collection ended there.isoOrNullaccepts anythingDate.parsetakes, buttimelineReplySchema.published_atisz.iso.datetime(); normalising vianew Date(v).toISOString()would make the DTO match its own schema.get_timeline_repliesreturnsjsonResponse({ error: 'Not found', success: false })where every other tool infeed-tools.tsuseserrorResponse(...).${origin}/users/${user}is now hand-built in three places (federation.ts:186,federation.ts:237,api.ts:405) next to the existingownObjectPrefixhelper; anownActorUri(origin, user)beside it would stop them drifting. Relatedly,ingestNoteForRecipient'sorigin?is optional but both callers now pass it, so theorigin == null → mentions_me = falsebranch is dead.- Product note, not a defect: involvement now notifies regardless of the per-account bell, so a followee whose bell was explicitly switched off will still raise a notification when they reply to or mention you.
docs/features/feed.mdstates this, so I'm reading it as intentional.
A reply's byline came from remote-supplied attributedTo with no authority check, so a hostile origin (reachable by any stranger via mention ingestion) could render arbitrary text under any fediverse identity — a clean impersonation/phishing primitive that sanitisation can't touch. Replies are now held to the host that actually served them: inline items inherit the collection page's serving host, URI-referenced items their own URI's host (with the fetched document's id checked against it), and both the object's id and its attributedTo must live on that host or the reply is dropped — the same authority model noteToTimelineInput enforces on ingest. Tests cover the forged-byline inline item, the forged-id inline item, and the cross-host URI item. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwoP1SqJhHgHiEEoEQtUjT
fiddur
left a comment
There was a problem hiding this comment.
✅ Approved
Read the whole diff plus the surrounding ingest / SSRF / schema-migration code. Round 3 closes the earlier rounds' issues cleanly and I found no blocking correctness or security problem.
What I checked and am satisfied with:
- Stranger admission (
ingestStrangerInvolvement): gated on reply-to-an-existing-own-post (prefix + UUID +getFeedPostById) or an explicitMentionof the recipient's actor URI; requires explicitattributedTomatching the signature-verifiedactivity.actorId, and re-verifies the fetched actor'sid. Combined withnoteToTimelineInput's host check, theobject_uriupsert key can't be hijacked to overwrite another actor's entry. remote-replies.ts: the serving-host authority model (PageCtx.host, inline items inherit the page host, URI items get their own) plus the id/attributedTohost checks intoReplyblock byline forgery;httpsOnlyandisoOrNullsanitize the two values the web feeds tohref/date-fns; every fetch goes throughsafeFetchGet(maxRedirects: 0, private-range blocklist, 1 MB cap) and every failure path is swallowed rather than thrown. The page walk terminates in every cycle shape I could construct — a repeatingnextURI drains the fetch budget, and parsed JSON can't self-reference.- Migrations:
timeline_entry_mentions/timeline_entry_reply_checked/..._backstampare keys with no matching table name, so they run on the existing-DB path (!existingTableNames.has(tableName)) as well as on fresh init, in the right order relative totimeline_entry_replyand the indexes. The backstamp is genuinely idempotent because the upsert now always stampsreply_checked_at. - No-clobber backfill: candidates are
reply_checked_at IS NULL, which after the backstamp impliesin_reply_to_uri IS NULL, and a non-answer only callsmarkChecked— the new test asserts exactly that split. - Web:
dangerouslySetInnerHTMLis fed only server-sanitized HTML; the['userSettings']and['feed','timeline']query keys match the rest of the app;updateUserSettingsis a PATCH, so the single-field write doesn't clobber other settings. - Android:
decideNotifications' newinvolved || (top-level && bell on)rule is what the docs and the new test describe;fetchFollowingListswallows all exceptions, soAutoEnablePostNotifications'LaunchedEffectcan't crash the app when offline, and the launcher is registered before the effect runs.
Non-blocking — fold into a later PR, don't re-roll this one
Please do not push fixes for these here; another round costs more than they're worth. Batch them with other work:
- Two orphaned/stale doc comments. In
federation.tsthe oldingestFeedActivityblock ("Ingest aCreate/Update… Two admissible senders …") now sits directly above the newingestStrangerInvolvementdoc block, soingestFeedActivityitself is undocumented and the stacked block reads as describing the stranger helper. It is also now incomplete: it lists only "a reply to one of the recipient's own posts" and omits the Mention branch, as does the.on(Create, …)comment ("the only stranger Note admitted is a reply to one of the recipient's own posts"). Same shape intimeline-ingest.ts, where thenoteToTimelineInputdoc block (authority check /feed_followingpresentation) is now attached to the newly insertedTimelineAuthortype. Stale prose sitting on security-critical admission logic is the kind that misleads a later reader. - A couple of redundant comments.
// An inline item is held to the page's serving host; a URI item to its own.incollectPageRepliesrestates whatresolvePage's andtoReply's docs already say, and// Either way the choice is now recorded; only a grant starts the poller.inPostNotificationAutoEnable.ktnarrates the two lines under it. (The{/* Sanitised server-side … */}note inHomeTimeline.tsxis the opposite — keep that one.) partialover-reports. It is true wheneverreplies.length >= MAX_REPLIES, so a thread of exactly 20 is reported as truncated, and an author lookup that exhausts the fetch budget marks the whole snapshot partial even when every reply was collected. Conversely, a post whose own fetch fails returnspartial: false, replies: [], which the web renders as the confident "No replies found on the origin." rather than as a failure.- Weak assertion in the budget test.
expect(replies.length).toBeLessThanOrEqual(20)in "caps at the reply budget and reports partial" would pass with zero replies;toBe(20)would actually pin the cap the test name claims. - Small consistency items. MCP
get_timeline_repliesreturnsjsonResponse({ error: 'Not found', success: false })for not-found while every other not-found infeed-tools.tsuseserrorResponse(...); the 12 s budget isREPLIES_TIMEOUT_MSinfeed-router.tsbut a bare12_000literal in the MCP tool; and the reader's own actor URI is now built inline in three places (ingestNoteForRecipient,ingestStrangerInvolvement,api.ts) as${origin}/users/${user}whileownObjectPrefixusesencodeURIComponent(user)— harmless today givenisValidUsername, but a one-lineownActorUrihelper next toownObjectPrefixwould stop them drifting. - No unit test for the stranger-admission branch.
noteMentionsActorandingestStrangerInvolvementare the highest-risk new logic and are covered only indirectly.timeline-ingest.test.tsalready builds fedifyNotes, so "Mention of me admits / Mention of someone else doesn't / missingattributedTodrops" would be cheap insurance.
One design point worth being deliberate about rather than a defect: invalidating ['feed','timeline'] after the settings toggle prefix-matches every ['feed','timeline',<id>,'replies'] query, so each expanded thread refetches — up to 15 outbound requests apiece — on a checkbox flip.
Round 2 on #1060, from Fredrik's field feedback (with screenshots) + his follow-up question "are mentions of me from strangers also dropped?" (they were):
Why the timeline was still full of replies
Entries ingested before #1061 have no
in_reply_to_uri— the reply link was discarded at ingest — so the filter can't see them. Timeline reads now lazily backfill a few unchecked legacy entries per read (AP re-fetch of the object, one attempt, stamped via newreply_checked_at; fresh ingests stamp immediately). His timeline converges clean over a few page loads.Where the toggle is
timeline_show_repliesnow also sits on the Feed page next to the Home timeline (it stays in Settings too). Saving invalidates the timeline query so it applies immediately.Involvement: mentions + strangers
The inbox previously dropped every Note from a non-followee — including replies to your own posts and posts mentioning you. Now a non-followee's Note is admitted iff you're involved: it replies to one of your own (still existing) posts, or carries a
Mentionof your actor (newmentions_mecolumn/field; author snapshot from the signature-verified sender). Involved posts always show (marked "replied to you" / "mentioned you") and always notify — regardless of the reply setting or the per-follow bells. Everything else from strangers is still dropped.Expand replies
GET /feed/timeline/:id/replies+ MCPget_timeline_replies: a live, bounded snapshot of the post's remote AS2repliescollection — ≤20 replies from ≤15 SSRF-guarded fetches in a 12s budget, authors resolved + memoised, HTML sanitised server-side, nothing stored,partialflag when the thread outruns the budget. Web timeline cards get a Show replies button rendering it inline.Bells can't lie anymore
On launch,
AutoEnablePostNotificationsreconciles the Feed page's per-account 🔔 bells with the device: any bell on + no explicit on/off choice ever made → requestPOST_NOTIFICATIONSand start the poller. An explicit "off" is never overridden; a denial records "off" (no re-prompt loop) and the Account screen's existing blocked-warning takes over.Tests: 5 new remote-replies unit tests (page walk, URI items, budget/partial, sanitisation, author memoisation), parseReplyInfo/backfill tests, db integration for the mentions filter + backfill helpers +
getTimelineEntryById, Android involvement-predicate tests; whole-monorepo check green, activitypub suite 156/156, Android compile+unit green locally.🤖 Generated with Claude Code
https://claude.ai/code/session_01CwoP1SqJhHgHiEEoEQtUjT