Tighten log sanitization coverage

Author: gildesmarais

app/web/security/log_sanitizer.rb

@@ -47,13 +47,20 @@ def feed_suffix(path)
         # @param value [Object]
         # @return [Object]
         def sanitize_value(key, value)
-          return sanitize_url(value) if key.to_sym == :url
           return sanitize_details(value) if value.is_a?(Hash)
           return value.map { |entry| sanitize_value(key, entry) } if value.is_a?(Array)
+          return sanitize_url(value) if url_key?(key)
 
           value
         end
 
+        # @param key [Object]
+        # @return [Boolean]
+        def url_key?(key)
+          key_name = key.to_s
+          key_name == 'url' || key_name.end_with?('_url', '_urls')
+        end
+
         # @param value [Object]
         # @return [Hash{Symbol=>Object}, Object]
         def sanitize_url(value)

spec/html2rss/web/boot/setup_spec.rb

@@ -5,19 +5,30 @@
 require_relative '../../../../app'
 
 RSpec.describe Html2rss::Web::Boot::Setup do
-  describe '.call!' do
-    before do
-      allow(Html2rss::Web::EnvironmentValidator).to receive(:validate_environment!)
-      allow(Html2rss::Web::EnvironmentValidator).to receive(:validate_production_security!)
-      allow(Html2rss::Web::Flags).to receive(:validate!)
-    end
+  before do
+    allow(Html2rss::Web::EnvironmentValidator).to receive(:validate_environment!)
+    allow(Html2rss::Web::EnvironmentValidator).to receive(:validate_production_security!)
+    allow(Html2rss::Web::Flags).to receive(:validate!)
+  end
 
+  describe '.call!' do
     it 'validates environment state', :aggregate_failures do
       described_class.call!
 
       expect(Html2rss::Web::EnvironmentValidator).to have_received(:validate_environment!).once
       expect(Html2rss::Web::EnvironmentValidator).to have_received(:validate_production_security!).once
       expect(Html2rss::Web::Flags).to have_received(:validate!).once
     end
+
+    it 'routes rack-timeout logs through the shared app logger' do
+      stub_const('Rack::Timeout::Logger', Class.new)
+      logger_holder = { value: nil }
+      Rack::Timeout::Logger.define_singleton_method(:logger=) { |value| logger_holder[:value] = value }
+      Rack::Timeout::Logger.define_singleton_method(:logger) { logger_holder[:value] }
+
+      described_class.call!
+
+      expect(Rack::Timeout::Logger.logger).to be(Html2rss::Web::AppLogger.logger)
+    end
   end
 end

spec/html2rss/web/log_sanitizer_spec.rb

@@ -48,7 +48,7 @@
     expected_url = {
       host: 'news.ycombinator.com',
       scheme: 'https',
-      hash: Digest::SHA256.hexdigest('https://news.ycombinator.com')[0..11]
+      hash: url_hash('https://news.ycombinator.com')
     }
 
     expect(described_class.sanitize_details(url: 'https://news.ycombinator.com')).to eq(url: expected_url)
@@ -60,6 +60,14 @@
     )
   end
 
+  it 'sanitizes nested url fields when emitting shared log events' do
+    Html2rss::Web::LogEvent.emit(payload: nested_url_payload)
+
+    payload = JSON.parse(io.string.lines.last, symbolize_names: true)
+
+    expect(payload.slice(:url, :related_urls, :details)).to eq(expected_nested_url_payload)
+  end
+
   it 'sanitizes security logger token usage fields' do
     Html2rss::Web::SecurityLogger.log_token_usage('very-secret-token', 'https://news.ycombinator.com', true)
     payload = JSON.parse(io.string.lines.last, symbolize_names: true)
@@ -69,7 +77,7 @@
       url: {
         host: 'news.ycombinator.com',
         scheme: 'https',
-        hash: Digest::SHA256.hexdigest('https://news.ycombinator.com')[0..11]
+        hash: url_hash('https://news.ycombinator.com')
       },
       token_hash: Digest::SHA256.hexdigest('very-secret-token')[0..7]
     )
@@ -88,7 +96,7 @@
     expect(observability_payload.dig(:details, :url)).to eq(
       host: 'news.ycombinator.com',
       scheme: 'https',
-      hash: Digest::SHA256.hexdigest('https://news.ycombinator.com')[0..11]
+      hash: url_hash('https://news.ycombinator.com')
     )
   end
 
@@ -103,4 +111,41 @@
       state: 'completed'
     )
   end
+
+  private
+
+  # @return [Hash{Symbol=>Object}]
+  def nested_url_payload
+    {
+      url: 'https://news.ycombinator.com',
+      related_urls: ['https://example.com/feed.xml'],
+      details: { url: 'https://lobste.rs/s/test' }
+    }
+  end
+
+  # @return [Hash{Symbol=>Object}]
+  def expected_nested_url_payload
+    {
+      url: sanitized_url('news.ycombinator.com', 'https://news.ycombinator.com'),
+      related_urls: [
+        sanitized_url('example.com', 'https://example.com/feed.xml')
+      ],
+      details: {
+        url: sanitized_url('lobste.rs', 'https://lobste.rs/s/test')
+      }
+    }
+  end
+
+  # @param host [String]
+  # @param url [String]
+  # @return [Hash{Symbol=>String}]
+  def sanitized_url(host, url)
+    { host:, scheme: 'https', hash: url_hash(url) }
+  end
+
+  # @param url [String]
+  # @return [String]
+  def url_hash(url)
+    Digest::SHA256.hexdigest(url)[0..11]
+  end
 end