From 836a2bfee00de71e4943a99a7b8cfd2ce585f0c5 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 7 Aug 2025 14:52:29 +0200 Subject: [PATCH] Fix handling of inlined `instrument` in incoming `QuoteRequest` (#35714) --- app/lib/activitypub/activity/quote_request.rb | 25 ++++-- .../activity/quote_request_spec.rb | 80 +++++++++++++------ 2 files changed, 76 insertions(+), 29 deletions(-) diff --git a/app/lib/activitypub/activity/quote_request.rb b/app/lib/activitypub/activity/quote_request.rb index 9f18a1367b5..088360ff981 100644 --- a/app/lib/activitypub/activity/quote_request.rb +++ b/app/lib/activitypub/activity/quote_request.rb @@ -19,13 +19,13 @@ class ActivityPub::Activity::QuoteRequest < ActivityPub::Activity private def accept_quote_request!(quoted_status) - status = status_from_uri(@json['instrument']) - # TODO: import inlined quote post if possible - status ||= ActivityPub::FetchRemoteStatusService.new.call(@json['instrument'], on_behalf_of: quoted_status.account, request_id: @options[:request_id]) + status = status_from_uri(instrument_uri) + status ||= import_instrument(quoted_status) + status ||= ActivityPub::FetchRemoteStatusService.new.call(instrument_uri, on_behalf_of: quoted_status.account, request_id: @options[:request_id]) # TODO: raise if status is nil # Sanity check - return unless status.quote.quoted_status == quoted_status + return unless status.quote.quoted_status == quoted_status && status.account == @account status.quote.ensure_quoted_access status.quote.update!(activity_uri: @json['id']) @@ -35,15 +35,30 @@ class ActivityPub::Activity::QuoteRequest < ActivityPub::Activity ActivityPub::DeliveryWorker.perform_async(json, quoted_status.account_id, @account.inbox_url) end + def import_instrument(quoted_status) + return unless @json['instrument'].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 + instrument = @json['instrument'].merge({ '@context' => @json['@context'] }) + return if non_matching_uri_hosts?(instrument['id'], @account.uri) + + ActivityPub::FetchRemoteStatusService.new.call(instrument['id'], prefetched_body: instrument, on_behalf_of: quoted_status.account, request_id: @options[:request_id]) + end + def reject_quote_request!(quoted_status) quote = Quote.new( quoted_status: quoted_status, quoted_account: quoted_status.account, - status: Status.new(account: @account, uri: @json['instrument']), + status: Status.new(account: @account, uri: instrument_uri), account: @account, activity_uri: @json['id'] ) json = Oj.dump(serialize_payload(quote, ActivityPub::RejectQuoteRequestSerializer)) ActivityPub::DeliveryWorker.perform_async(json, quoted_status.account_id, @account.inbox_url) end + + def instrument_uri + value_or_id(@json['instrument']) + end end diff --git a/spec/lib/activitypub/activity/quote_request_spec.rb b/spec/lib/activitypub/activity/quote_request_spec.rb index 24a245c8981..64627cbdfbe 100644 --- a/spec/lib/activitypub/activity/quote_request_spec.rb +++ b/spec/lib/activitypub/activity/quote_request_spec.rb @@ -8,6 +8,7 @@ RSpec.describe ActivityPub::Activity::QuoteRequest, feature: :outgoing_quotes do let(:quoted_post) { Fabricate(:status, account: recipient) } let(:request_uri) { 'https://example.com/missing-ui' } let(:quoted_uri) { ActivityPub::TagManager.instance.uri_for(quoted_post) } + let(:instrument) { 'https://example.com/unknown-status' } let(:json) do { @@ -21,8 +22,30 @@ RSpec.describe ActivityPub::Activity::QuoteRequest, feature: :outgoing_quotes do type: 'QuoteRequest', actor: ActivityPub::TagManager.instance.uri_for(sender), object: quoted_uri, - instrument: 'https://example.com/unknown-status', - }.with_indifferent_access + instrument: instrument, + }.deep_stringify_keys + end + + let(:status_json) do + { + '@context': [ + 'https://www.w3.org/ns/activitystreams', + { + '@id': 'https://w3id.org/fep/044f#quote', + '@type': '@id', + }, + { + '@id': 'https://w3id.org/fep/044f#quoteAuthorization', + '@type': '@id', + }, + ], + id: 'https://example.com/unknown-status', + type: 'Note', + summary: 'Show more', + content: 'Hello universe', + quote: ActivityPub::TagManager.instance.uri_for(quoted_post), + attributedTo: ActivityPub::TagManager.instance.uri_for(sender), + }.deep_stringify_keys end describe '#perform' do @@ -48,29 +71,20 @@ RSpec.describe ActivityPub::Activity::QuoteRequest, feature: :outgoing_quotes do end end - context 'when trying to quote a quotable local status' do - let(:status_json) do - { - '@context': [ - 'https://www.w3.org/ns/activitystreams', - { - '@id': 'https://w3id.org/fep/044f#quote', - '@type': '@id', - }, - { - '@id': 'https://w3id.org/fep/044f#quoteAuthorization', - '@type': '@id', - }, - ], - id: 'https://example.com/unknown-status', - type: 'Note', - summary: 'Show more', - content: 'Hello universe', - quote: ActivityPub::TagManager.instance.uri_for(quoted_post), - attributedTo: ActivityPub::TagManager.instance.uri_for(sender), - }.deep_stringify_keys - end + context 'when trying to quote an unquotable local status with an inlined instrument' do + let(:instrument) { status_json.without('@context') } + it 'sends a Reject activity' do + expect { subject.perform } + .to enqueue_sidekiq_job(ActivityPub::DeliveryWorker) + .with(satisfying do |body| + outgoing_json = Oj.load(body) + outgoing_json['type'] == 'Reject' && json['instrument']['id'] == outgoing_json['object']['instrument'] && %w(type id actor object).all? { |key| json[key] == outgoing_json['object'][key] } + end, recipient.id, sender.inbox_url) + end + end + + context 'when trying to quote a quotable local status' do before do stub_request(:get, 'https://example.com/unknown-status').to_return(status: 200, body: Oj.dump(status_json), headers: { 'Content-Type': 'application/activity+json' }) quoted_post.update(quote_approval_policy: Status::QUOTE_APPROVAL_POLICY_FLAGS[:public] << 16) @@ -86,5 +100,23 @@ RSpec.describe ActivityPub::Activity::QuoteRequest, feature: :outgoing_quotes do end, recipient.id, sender.inbox_url) end end + + context 'when trying to quote a quotable local status with an inlined instrument' do + let(:instrument) { status_json.without('@context') } + + before do + quoted_post.update(quote_approval_policy: Status::QUOTE_APPROVAL_POLICY_FLAGS[:public] << 16) + end + + it 'accepts the quote and sends an Accept activity' do + expect { subject.perform } + .to change { quoted_post.reload.quotes.accepted.count }.by(1) + .and enqueue_sidekiq_job(ActivityPub::DeliveryWorker) + .with(satisfying do |body| + outgoing_json = Oj.load(body) + outgoing_json['type'] == 'Accept' && json['instrument']['id'] == outgoing_json['object']['instrument'] && %w(type id actor object).all? { |key| json[key] == outgoing_json['object'][key] } + end, recipient.id, sender.inbox_url) + end + end end end