harden feed responder and resolver

Author: gildesmarais

app/feeds/responder.rb

@@ -27,9 +27,10 @@ def call(request:, target_kind:, identifier:)
             resolved_source = SourceResolver.call(feed_request)
             result = Service.call(resolved_source)
             normalized_identifier = feed_request.feed_name || identifier
+            body = write_response(response: request.response, representation: feed_request.representation, result:)
 
             emit_result(target_kind:, identifier: normalized_identifier, resolved_source:, result:)
-            write_response(response: request.response, representation: feed_request.representation, result:)
+            body
           rescue StandardError => error
             emit_failure(target_kind:, identifier:, error:)
             raise

app/feeds/source_resolver.rb

@@ -37,14 +37,13 @@ def call(feed_request)
           # @return [Html2rss::Web::Feeds::Contracts::ResolvedSource]
           def resolve_static(feed_request)
             config = LocalConfig.find(feed_request.feed_name)
-            config[:params] = (config[:params] || {}).merge(feed_request.params) if feed_request.params.any?
-            config[:strategy] ||= Html2rss::RequestService.default_strategy_name
+            generator_input = static_generator_input(config, feed_request.params)
 
             Contracts::ResolvedSource.new(
               source_kind: :static,
               cache_identity: static_cache_identity(feed_request.feed_name, feed_request.params),
-              generator_input: config,
-              ttl_seconds: CacheTtl.seconds_from_minutes(config.dig(:channel, :ttl))
+              generator_input: generator_input,
+              ttl_seconds: CacheTtl.seconds_from_minutes(generator_input.dig(:channel, :ttl))
             )
           end
 
@@ -73,6 +72,25 @@ def static_cache_identity(feed_name, params)
             "static:#{feed_name}:#{digest}"
           end
 
+          # @param config [Hash{Symbol=>Object}]
+          # @param params [Hash{Object=>Object}]
+          # @return [Hash{Symbol=>Object}]
+          def static_generator_input(config, params)
+            generator_input = config.dup
+            generator_input[:params] = merged_static_params(config, params)
+            generator_input[:strategy] ||= Html2rss::RequestService.default_strategy_name
+            generator_input
+          end
+
+          # @param config [Hash{Symbol=>Object}]
+          # @param params [Hash{Object=>Object}]
+          # @return [Hash{Object=>Object}]
+          def merged_static_params(config, params)
+            return (config[:params] || {}).dup if params.empty?
+
+            (config[:params] || {}).merge(params)
+          end
+
           # @param token [String]
           # @return [String]
           def token_cache_identity(token)

spec/html2rss/web/feeds/responder_spec.rb

@@ -65,6 +65,17 @@ def resolved_source
 
       expect(response['Cache-Control']).to include('public')
     end
+
+    it 'emits success after writing the response' do
+      write_response
+
+      expect(Html2rss::Web::Observability).to have_received(:emit).with(
+        event_name: 'feed.render',
+        outcome: 'success',
+        details: include(strategy: :ssrf_filter, url: 'https://example.com'),
+        level: :info
+      )
+    end
   end
 
   context 'with an error result' do
@@ -108,6 +119,48 @@ def resolved_source
     end
   end
 
+  context 'when response rendering fails after feed generation succeeds' do
+    subject(:write_response) do
+      described_class.call(
+        request: request,
+        target_kind: :token,
+        identifier: 'token'
+      )
+    end
+
+    let(:representation) { Html2rss::Web::FeedResponseFormat::RSS }
+
+    let(:result) do
+      Html2rss::Web::Feeds::Contracts::RenderResult.new(
+        status: :ok,
+        payload: nil,
+        message: nil,
+        ttl_seconds: 600,
+        cache_key: 'feed_result:test',
+        error_message: nil
+      )
+    end
+
+    before do
+      allow(Html2rss::Web::Feeds::Request).to receive(:call).and_return(feed_request(representation))
+      allow(Html2rss::Web::Feeds::SourceResolver).to receive(:call).and_return(resolved_source)
+      allow(Html2rss::Web::Feeds::Service).to receive(:call).and_return(result)
+      allow(Html2rss::Web::Observability).to receive(:emit)
+      allow(Html2rss::Web::Feeds::RssRenderer).to receive(:call).and_raise(StandardError, 'render failed')
+    end
+
+    it 'emits only the failure event' do
+      expect { write_response }.to raise_error(StandardError, 'render failed')
+
+      expect(Html2rss::Web::Observability).to have_received(:emit).once.with(
+        event_name: 'feed.render',
+        outcome: 'failure',
+        details: include(error_class: 'StandardError', error_message: 'render failed'),
+        level: :warn
+      )
+    end
+  end
+
   private
 
   # @param body [String]

spec/html2rss/web/feeds/source_resolver_spec.rb

@@ -10,6 +10,13 @@ def resolved_tuple(resolved)
     end
 
     context 'with a static request' do
+      let(:config) do
+        {
+          channel: { ttl: 15, url: 'https://example.com/feed' },
+          params: { 'existing' => '1' }
+        }
+      end
+
       let(:feed_request) do
         Html2rss::Web::Feeds::Contracts::Request.new(
           target_kind: :static,
@@ -21,12 +28,7 @@ def resolved_tuple(resolved)
       end
 
       before do
-        allow(Html2rss::Web::LocalConfig).to receive(:find).with('legacy').and_return(
-          {
-            channel: { ttl: 15, url: 'https://example.com/feed' },
-            params: { 'existing' => '1' }
-          }
-        )
+        allow(Html2rss::Web::LocalConfig).to receive(:find).with('legacy').and_return(config)
         allow(Html2rss::RequestService).to receive(:default_strategy_name).and_return(:ssrf_filter)
       end
 
@@ -42,6 +44,15 @@ def resolved_tuple(resolved)
           ]
         )
       end
+
+      it 'does not mutate the source config hash' do
+        described_class.call(feed_request)
+
+        expect(config).to eq(
+          channel: { ttl: 15, url: 'https://example.com/feed' },
+          params: { 'existing' => '1' }
+        )
+      end
     end
 
     context 'with a token request' do