fix: harden feed loading and negotiated openapi docs

Author: gildesmarais

app/feeds/cache.rb

@@ -21,21 +21,25 @@ class << self
           # @yieldreturn [Html2rss::Web::Feeds::Result]
           # @return [Html2rss::Web::Feeds::Result]
           def fetch(key, ttl_seconds:, cacheable: true)
-            entry = read_entry(key)
-            return entry.result if fresh?(entry)
+            lock.synchronize do
+              entry = read_entry(key)
+              return entry.result if fresh?(entry)
 
-            result = yield
-            return result unless cacheable_result?(cacheable, result)
+              result = yield
+              return result unless cacheable_result?(cacheable, result)
 
-            write_entry(key, ttl_seconds, result)
-            result
+              write_entry(key, ttl_seconds, result)
+              result
+            end
           end
 
           # @param reason [String]
           # @return [nil]
           def clear!(reason: 'manual')
-            @entries = {} # rubocop:disable ThreadSafety/ClassInstanceVariable
-            SecurityLogger.log_cache_lifecycle('feeds_cache', 'clear', reason: reason)
+            lock.synchronize do
+              @entries = {}
+              SecurityLogger.log_cache_lifecycle('feeds_cache', 'clear', reason: reason)
+            end
             nil
           end
 
@@ -76,6 +80,11 @@ def entries
             @entries ||= {} # rubocop:disable ThreadSafety/ClassInstanceVariable
           end
 
+          # @return [Mutex]
+          def lock
+            @lock ||= Mutex.new # rubocop:disable ThreadSafety/ClassInstanceVariable
+          end
+
           # @param ttl_seconds [Integer]
           # @return [Integer]
           def normalize_ttl(ttl_seconds)

app/feeds/service.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require_relative '../exceptions'
 require_relative 'cache'
 require_relative 'payload'
 require_relative 'result'

app/http/feed_route_handler.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require_relative '../exceptions'
 require_relative '../feed_response_format'
 require_relative '../feeds/json_renderer'
 require_relative '../feeds/request_parser'

docs/api/v1/openapi.yaml

@@ -192,25 +192,56 @@ paths:
       responses:
         '200':
           content:
+            application/feed+json:
+              schema:
+                type: string
             application/xml:
               schema:
                 type: string
           description: renders feed for a valid token
         '401':
           content:
             application/feed+json:
+              example:
+                title: Error
+                version: https://jsonfeed.org/version/1.1
+              schema:
+                type: string
+            application/xml:
+              example: |-
+                <?xml version="1.0" encoding="UTF-8"?>
+                <rss version="2.0"><channel><title>Error</title><description>Internal Server Error</description></channel></rss>
               schema:
                 type: string
           description: returns JSON Feed-shaped errors when requested by json extension
         '403':
           content:
+            application/feed+json:
+              example:
+                title: Error
+                version: https://jsonfeed.org/version/1.1
+              schema:
+                type: string
             application/xml:
+              example: |-
+                <?xml version="1.0" encoding="UTF-8"?>
+                <rss version="2.0"><channel><title>Error</title><description>Internal Server Error</description></channel></rss>
               schema:
                 type: string
-          description: returns forbidden when auto source is disabled
+          description: returns JSON Feed-shaped forbidden errors when requested through
+            Accept
         '500':
           content:
             application/feed+json:
+              example:
+                title: Error
+                version: https://jsonfeed.org/version/1.1
+              schema:
+                type: string
+            application/xml:
+              example: |-
+                <?xml version="1.0" encoding="UTF-8"?>
+                <rss version="2.0"><channel><title>Error</title><description>Internal Server Error</description></channel></rss>
               schema:
                 type: string
           description: returns non-cacheable json feed errors when service generation

spec/html2rss/web/api/v1_spec.rb

@@ -224,7 +224,8 @@ def json_feed_headers_tuple
     summary: 'Render feed by token',
     operation_id: 'renderFeedByToken',
     tags: ['Feeds'],
-    security: []
+    security: [],
+    example_mode: :multiple
   } do
     before do
       stub_const('Html2rss::FeedChannel', Class.new { attr_reader :ttl })
@@ -312,8 +313,36 @@ def json_feed_headers_tuple
       expect(last_response.body).to include(Html2rss::Web::Api::V1::Contract::MESSAGES[:auto_source_disabled])
     end
 
+    it 'returns JSON Feed-shaped forbidden errors when requested through Accept', :aggregate_failures do
+      unique_url = "#{feed_url}/disabled-json"
+      token = Html2rss::Web::Auth.generate_feed_token('admin', unique_url, strategy: 'ssrf_filter')
+
+      ClimateControl.modify(AUTO_SOURCE_ENABLED: 'false') do
+        get "/api/v1/feeds/#{token}", {}, { 'HTTP_ACCEPT' => 'application/feed+json' }
+      end
+
+      expect([last_response.status, last_response.headers['Content-Type'], json_feed_error]).to eq(
+        [403, 'application/feed+json', { 'version' => 'https://jsonfeed.org/version/1.1', 'title' => 'Error' }]
+      )
+    end
+
+    it 'returns non-cacheable xml feed errors when service generation fails', :aggregate_failures do
+      unique_url = "#{feed_url}/service-error-xml"
+      token = Html2rss::Web::Auth.generate_feed_token('admin', unique_url, strategy: 'ssrf_filter')
+
+      allow(Html2rss::Web::Feeds::Service).to receive(:call).and_return(service_error_result)
+
+      get "/api/v1/feeds/#{token}.xml"
+
+      expect(last_response.status).to eq(500)
+      expect(last_response.content_type).to include('application/xml')
+      expect(last_response.headers['Cache-Control']).to include('no-store')
+      expect(last_response.body).to include('Internal Server Error')
+    end
+
     it 'returns non-cacheable json feed errors when service generation fails', :aggregate_failures do
-      token = Html2rss::Web::Auth.generate_feed_token('admin', feed_url, strategy: 'ssrf_filter')
+      unique_url = "#{feed_url}/service-error-json"
+      token = Html2rss::Web::Auth.generate_feed_token('admin', unique_url, strategy: 'ssrf_filter')
 
       status, content_type, cache_control, title = json_feed_service_error_tuple(token)
 

spec/support/openapi.rb

@@ -47,6 +47,8 @@
 
 # Keep path keys relative to /api/v1 because servers include the versioned base path.
 RSpec::OpenAPI.post_process_hook = lambda do |_path, _records, spec|
+  token_feed_error_statuses = %w[401 403 500].freeze
+
   stringify = lambda do |value|
     case value
     when Hash
@@ -69,6 +71,51 @@
     end
   end
 
+  merge_responses = lambda do |existing_responses, new_responses|
+    statuses = existing_responses.keys | new_responses.keys
+
+    statuses.each_with_object({}) do |status, merged_responses|
+      current = existing_responses[status] || {}
+      incoming = new_responses[status] || {}
+      merged_response = current.merge(incoming)
+
+      current_content = current['content'] || {}
+      incoming_content = incoming['content'] || {}
+      if current_content.any? || incoming_content.any?
+        content_types = current_content.keys | incoming_content.keys
+        merged_response['content'] = content_types.to_h do |content_type|
+          current_entry = current_content[content_type] || {}
+          incoming_entry = incoming_content[content_type] || {}
+          [content_type, current_entry.merge(incoming_entry)]
+        end
+      end
+
+      current_headers = current['headers'] || {}
+      incoming_headers = incoming['headers'] || {}
+      if current_headers.any? || incoming_headers.any?
+        merged_response['headers'] = current_headers.merge(incoming_headers)
+      end
+
+      merged_response['description'] ||= current['description'] || incoming['description']
+      merged_responses[status] = merged_response
+    end
+  end
+
+  token_feed_error_examples = {
+    'application/xml' => {
+      'example' => <<~XML.strip
+        <?xml version="1.0" encoding="UTF-8"?>
+        <rss version="2.0"><channel><title>Error</title><description>Internal Server Error</description></channel></rss>
+      XML
+    },
+    'application/feed+json' => {
+      'example' => {
+        'version' => 'https://jsonfeed.org/version/1.1',
+        'title' => 'Error'
+      }
+    }
+  }
+
   path_map = spec['paths'] || spec[:paths]
   next unless path_map.is_a?(Hash)
 
@@ -90,7 +137,7 @@
 
       if existing
         merged = existing.merge(operation_doc)
-        merged['responses'] = (existing['responses'] || {}).merge(operation_doc['responses'] || {})
+        merged['responses'] = merge_responses.call(existing['responses'] || {}, operation_doc['responses'] || {})
         merged['parameters'] = [*(existing['parameters'] || []), *(operation_doc['parameters'] || [])]
         merged['parameters'].uniq! { |parameter| [parameter['name'], parameter['in']] }
         normalized_paths[normalized][verb] = deep_sort.call(merged)
@@ -106,14 +153,25 @@
       has_token_param = normalized_paths[normalized][verb]['parameters'].any? do |parameter|
         parameter['name'] == 'token' && parameter['in'] == 'path'
       end
-      next if has_token_param
+      unless has_token_param
+        normalized_paths[normalized][verb]['parameters'] << {
+          'name' => 'token',
+          'in' => 'path',
+          'required' => true,
+          'schema' => { 'type' => 'string' }
+        }
+      end
 
-      normalized_paths[normalized][verb]['parameters'] << {
-        'name' => 'token',
-        'in' => 'path',
-        'required' => true,
-        'schema' => { 'type' => 'string' }
-      }
+      token_feed_error_statuses.each do |status|
+        response = normalized_paths[normalized][verb].dig('responses', status)
+        next unless response
+
+        response['content'] ||= {}
+        token_feed_error_examples.each do |content_type, example|
+          response['content'][content_type] ||= { 'schema' => { 'type' => 'string' } }
+          response['content'][content_type].merge!(example)
+        end
+      end
     end
   end