From c2ba6b10d7a2ee9b641690f31cd87a4a448fd6ce Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 30 Apr 2025 15:25:46 +0200 Subject: [PATCH 1/3] Refactor status update quote processing --- .../process_status_update_service.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index e02cc0914b..8d746b82a5 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -273,7 +273,6 @@ class ActivityPub::ProcessStatusUpdateService < BaseService def update_quote! return unless Mastodon::Feature.inbound_quotes_enabled? - quote = nil quote_uri = @status_parser.quote_uri if quote_uri.present? @@ -294,21 +293,22 @@ class ActivityPub::ProcessStatusUpdateService < BaseService quote = Quote.create(status: @status, approval_uri: approval_uri) @quote_changed = true end - end - if quote.present? - begin - quote.save - ActivityPub::VerifyQuoteService.new.call(quote, fetchable_quoted_uri: quote_uri, request_id: @request_id) - rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS - ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, quote.id, quote_uri, { 'request_id' => @request_id }) - end + quote.save + + fetch_and_verify_quote!(quote, quote_uri) elsif @status.quote.present? @status.quote.destroy! @quote_changed = true end end + def fetch_and_verify_quote!(quote, quote_uri) + ActivityPub::VerifyQuoteService.new.call(quote, fetchable_quoted_uri: quote_uri, request_id: @request_id) + rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS + ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, quote.id, quote_uri, { 'request_id' => @request_id }) + end + def update_counts! likes = @status_parser.favourites_count shares = @status_parser.reblogs_count From 7ae47b974b1cb419d498885ecfad50f1e0793e11 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 30 Apr 2025 16:37:48 +0200 Subject: [PATCH 2/3] Add support for importing inlined self-quotes --- app/lib/activitypub/activity/create.rb | 16 ++++++++- app/lib/activitypub/parser/status_parser.rb | 5 +++ .../process_status_update_service.rb | 15 ++++++++- .../activitypub/verify_quote_service.rb | 10 +++--- .../activitypub/verify_quote_service_spec.rb | 33 ++++++++++++++++++- 5 files changed, 71 insertions(+), 8 deletions(-) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 1710fc80b4..97d929d768 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -204,11 +204,25 @@ class ActivityPub::Activity::Create < ActivityPub::Activity @quote.status = status @quote.save - ActivityPub::VerifyQuoteService.new.call(@quote, fetchable_quoted_uri: @quote_uri, request_id: @options[:request_id]) + + ActivityPub::VerifyQuoteService.new.call(@quote, fetchable_quoted_uri: @quote_uri, prefetched_quoted_object: safe_prefetched_quoted_object, request_id: @options[:request_id]) rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, @quote.id, @quote_uri, { 'request_id' => @options[:request_id] }) end + def safe_prefetched_quoted_object + object = @status_parser.quoted_object + return unless object.is_a?(Hash) + + # NOTE: Replacing the object's context by that of the parent activity is + # not sound, but it's consistent with the rest of the codebase + object = object.merge({ '@context' => @json['@context'] }) + + return if value_or_id(first_of_value(object['attributedTo'])) != @account.uri || non_matching_uri_hosts?(@account.uri, object['id']) + + object + end + def process_tags return if @object['tag'].nil? diff --git a/app/lib/activitypub/parser/status_parser.rb b/app/lib/activitypub/parser/status_parser.rb index f66cd4aab0..f60a314c48 100644 --- a/app/lib/activitypub/parser/status_parser.rb +++ b/app/lib/activitypub/parser/status_parser.rb @@ -120,6 +120,11 @@ class ActivityPub::Parser::StatusParser end.first end + # The inlined quote; out of the attributes we support, only `https://w3id.org/fep/044f#quote` explicitly supports inlined objects + def quoted_object + as_array(@object['quote']).first + end + def quote_approval_uri as_array(@object['quoteAuthorization']).first end diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 8d746b82a5..c6d957f693 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -304,11 +304,24 @@ class ActivityPub::ProcessStatusUpdateService < BaseService end def fetch_and_verify_quote!(quote, quote_uri) - ActivityPub::VerifyQuoteService.new.call(quote, fetchable_quoted_uri: quote_uri, request_id: @request_id) + ActivityPub::VerifyQuoteService.new.call(quote, fetchable_quoted_uri: quote_uri, prefetched_quoted_object: safe_prefetched_quoted_object, request_id: @request_id) rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, quote.id, quote_uri, { 'request_id' => @request_id }) end + def safe_prefetched_quoted_object + object = @status_parser.quoted_object + return unless object.is_a?(Hash) + + # NOTE: Replacing the object's context by that of the parent activity is + # not sound, but it's consistent with the rest of the codebase + object = object.merge({ '@context' => @activity_json['@context'] }) + + return if value_or_id(first_of_value(object['attributedTo'])) != @account.uri || non_matching_uri_hosts?(@account.uri, object['id']) + + object + end + def update_counts! likes = @status_parser.favourites_count shares = @status_parser.reblogs_count diff --git a/app/services/activitypub/verify_quote_service.rb b/app/services/activitypub/verify_quote_service.rb index 0803d62d3a..ad4dfbe310 100644 --- a/app/services/activitypub/verify_quote_service.rb +++ b/app/services/activitypub/verify_quote_service.rb @@ -4,15 +4,15 @@ class ActivityPub::VerifyQuoteService < BaseService include JsonLdHelper # Optionally fetch quoted post, and verify the quote is authorized - def call(quote, fetchable_quoted_uri: nil, prefetched_body: nil, request_id: nil) + def call(quote, fetchable_quoted_uri: nil, prefetched_quoted_object: nil, prefetched_approval: nil, request_id: nil) @request_id = request_id @quote = quote @fetching_error = nil - fetch_quoted_post_if_needed!(fetchable_quoted_uri) + fetch_quoted_post_if_needed!(fetchable_quoted_uri, prefetched_body: prefetched_quoted_object) return if fast_track_approval! || quote.approval_uri.blank? - @json = fetch_approval_object(quote.approval_uri, prefetched_body:) + @json = fetch_approval_object(quote.approval_uri, prefetched_body: prefetched_approval) return quote.reject! if @json.nil? return if non_matching_uri_hosts?(quote.approval_uri, value_or_id(@json['attributedTo'])) @@ -68,11 +68,11 @@ class ActivityPub::VerifyQuoteService < BaseService ActivityPub::TagManager.instance.uri_for(@quote.status) == value_or_id(@json['interactingObject']) end - def fetch_quoted_post_if_needed!(uri) + def fetch_quoted_post_if_needed!(uri, prefetched_body: nil) return if uri.nil? || @quote.quoted_status.present? status = ActivityPub::TagManager.instance.uri_to_resource(uri, Status) - status ||= ActivityPub::FetchRemoteStatusService.new.call(uri, on_behalf_of: @quote.account.followers.local.first, request_id: @request_id) + status ||= ActivityPub::FetchRemoteStatusService.new.call(uri, on_behalf_of: @quote.account.followers.local.first, prefetched_body:, request_id: @request_id) @quote.update(quoted_status: status) if status.present? rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS => e diff --git a/spec/services/activitypub/verify_quote_service_spec.rb b/spec/services/activitypub/verify_quote_service_spec.rb index 0e5069a46b..ae4ffae9bb 100644 --- a/spec/services/activitypub/verify_quote_service_spec.rb +++ b/spec/services/activitypub/verify_quote_service_spec.rb @@ -89,6 +89,37 @@ RSpec.describe ActivityPub::VerifyQuoteService do end end + context 'with a valid activity for a post that cannot be fetched but is passed as fetched_quoted_object' do + let(:quoted_status) { nil } + + let(:approval_interaction_target) { 'https://b.example.com/unknown-quoted' } + let(:prefetched_object) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Note', + id: 'https://b.example.com/unknown-quoted', + to: 'https://www.w3.org/ns/activitystreams#Public', + attributedTo: ActivityPub::TagManager.instance.uri_for(quoted_account), + content: 'previously unknown post', + }.with_indifferent_access + end + + before do + stub_request(:get, 'https://b.example.com/unknown-quoted') + .to_return(status: 404) + end + + it 'updates the status' do + expect { subject.call(quote, fetchable_quoted_uri: 'https://b.example.com/unknown-quoted', prefetched_quoted_object: prefetched_object) } + .to change(quote, :state).to('accepted') + + expect(a_request(:get, approval_uri)) + .to have_been_made.once + + expect(quote.reload.quoted_status.content).to eq 'previously unknown post' + end + end + context 'with a valid activity for a post that cannot be fetched but is inlined' do let(:quoted_status) { nil } @@ -148,7 +179,7 @@ RSpec.describe ActivityPub::VerifyQuoteService do context 'with a valid activity for already-fetched posts, with a pre-fetched approval' do it 'updates the status without fetching the activity' do - expect { subject.call(quote, prefetched_body: Oj.dump(json)) } + expect { subject.call(quote, prefetched_approval: Oj.dump(json)) } .to change(quote, :state).to('accepted') expect(a_request(:get, approval_uri)) From a77d0fe7b8a9b5d68eb16acb324ef4ea0ea37e85 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 2 May 2025 17:29:48 +0200 Subject: [PATCH 3/3] Refactor `safe_prefetched_quoted_object` in `safe_prefetched_embed` helper --- app/helpers/json_ld_helper.rb | 12 ++++++++++++ app/lib/activitypub/activity/create.rb | 16 ++-------------- .../activitypub/process_status_update_service.rb | 16 ++-------------- 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/app/helpers/json_ld_helper.rb b/app/helpers/json_ld_helper.rb index 65daaa5302..212894d0cd 100644 --- a/app/helpers/json_ld_helper.rb +++ b/app/helpers/json_ld_helper.rb @@ -72,6 +72,18 @@ module JsonLdHelper !haystack.casecmp(needle).zero? end + def safe_prefetched_embed(account, object, context) + return unless object.is_a?(Hash) + + # NOTE: Replacing the object's context by that of the parent activity is + # not sound, but it's consistent with the rest of the codebase + object = object.merge({ '@context' => context }) + + return if value_or_id(first_of_value(object['attributedTo'])) != account.uri || non_matching_uri_hosts?(account.uri, object['id']) + + object + end + def canonicalize(json) graph = RDF::Graph.new << JSON::LD::API.toRdf(json, documentLoader: method(:load_jsonld_context)) graph.dump(:normalize) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 97d929d768..06bc940949 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -205,24 +205,12 @@ class ActivityPub::Activity::Create < ActivityPub::Activity @quote.status = status @quote.save - ActivityPub::VerifyQuoteService.new.call(@quote, fetchable_quoted_uri: @quote_uri, prefetched_quoted_object: safe_prefetched_quoted_object, request_id: @options[:request_id]) + embedded_quote = safe_prefetched_embed(@account, @status_parser.quoted_object, @json['context']) + ActivityPub::VerifyQuoteService.new.call(@quote, fetchable_quoted_uri: @quote_uri, prefetched_quoted_object: embedded_quote, request_id: @options[:request_id]) rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, @quote.id, @quote_uri, { 'request_id' => @options[:request_id] }) end - def safe_prefetched_quoted_object - object = @status_parser.quoted_object - return unless object.is_a?(Hash) - - # NOTE: Replacing the object's context by that of the parent activity is - # not sound, but it's consistent with the rest of the codebase - object = object.merge({ '@context' => @json['@context'] }) - - return if value_or_id(first_of_value(object['attributedTo'])) != @account.uri || non_matching_uri_hosts?(@account.uri, object['id']) - - object - end - def process_tags return if @object['tag'].nil? diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index c6d957f693..aceb17f2d1 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -304,24 +304,12 @@ class ActivityPub::ProcessStatusUpdateService < BaseService end def fetch_and_verify_quote!(quote, quote_uri) - ActivityPub::VerifyQuoteService.new.call(quote, fetchable_quoted_uri: quote_uri, prefetched_quoted_object: safe_prefetched_quoted_object, request_id: @request_id) + embedded_quote = safe_prefetched_embed(@account, @status_parser.quoted_object, @activity_json['context']) + ActivityPub::VerifyQuoteService.new.call(quote, fetchable_quoted_uri: quote_uri, prefetched_quoted_object: embedded_quote, request_id: @request_id) rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, quote.id, quote_uri, { 'request_id' => @request_id }) end - def safe_prefetched_quoted_object - object = @status_parser.quoted_object - return unless object.is_a?(Hash) - - # NOTE: Replacing the object's context by that of the parent activity is - # not sound, but it's consistent with the rest of the codebase - object = object.merge({ '@context' => @activity_json['@context'] }) - - return if value_or_id(first_of_value(object['attributedTo'])) != @account.uri || non_matching_uri_hosts?(@account.uri, object['id']) - - object - end - def update_counts! likes = @status_parser.favourites_count shares = @status_parser.reblogs_count