#132413: Implement SafeJsonFormatter for SemanticLogger (#27330)

* #132413: Implement SafeJsonFormatter for SemanticLogger

* create tests for formatter behavior

* Update CODEOWNERS

* fix spec/formatters directory

* add methods to SafeJsonFormatter class

* prepend SafeFormatter instantiation to prod and dev config

* move formatter from app/ to lib/

* Update CODEOWNERS

* lib/formatters -> lib/semantic_logger

* remove unused app/formatters/ folder

* fix typo in lib/semantic_logger/ entry

* remove unused spec/formatters folder

* fix typo in require_relative in application.rb

* add tests, expand rescue, sanitize keys

Author: Crankums

.github/CODEOWNERS

@@ -1127,6 +1127,7 @@ lib/search @department-of-veterans-affairs/backend-review-group
 lib/search/pii_redactor.rb @department-of-veterans-affairs/backend-review-group
 lib/search_click_tracking @department-of-veterans-affairs/backend-review-group
 lib/search_gsa @department-of-veterans-affairs/backend-review-group
+lib/semantic_logger/safe_json_formatter.rb @department-of-veterans-affairs/backend-review-group
 lib/sentry @department-of-veterans-affairs/backend-review-group
 lib/sftp_writer @department-of-veterans-affairs/backend-review-group
 lib/shrine @department-of-veterans-affairs/backend-review-group
@@ -1735,6 +1736,7 @@ spec/lib/search @department-of-veterans-affairs/backend-review-group
 spec/lib/search/pii_redactor_spec.rb @department-of-veterans-affairs/backend-review-group
 spec/lib/search_click_tracking @department-of-veterans-affairs/backend-review-group
 spec/lib/search_gsa @department-of-veterans-affairs/backend-review-group
+spec/lib/semantic_logger/safe_json_formatter_spec.rb @department-of-veterans-affairs/backend-review-group
 spec/lib/sentry @department-of-veterans-affairs/backend-review-group
 spec/lib/sftp_writer @department-of-veterans-affairs/backend-review-group
 spec/lib/sftp_writer/factory_spec.rb @department-of-veterans-affairs/backend-review-group

config/application.rb

@@ -18,7 +18,7 @@
 # Require the gems listed in Gemfile, including any gems
 # you've limited to :test, :development, or :production.
 Bundler.require(*Rails.groups)
-
+require_relative '../lib/semantic_logger/safe_json_formatter'
 require_relative '../lib/olive_branch_patch'
 
 module VetsAPI

config/environments/development.rb

@@ -90,6 +90,6 @@
   config.semantic_logger.add_appender(
     io: $stdout,
     level: :info, # For debug, see log/development.log
-    formatter: config.rails_semantic_logger.format
+    formatter: SafeJsonFormatter.new
   )
 end

config/environments/production.rb

@@ -75,7 +75,7 @@
   config.rails_semantic_logger.add_file_appender = false
   config.semantic_logger.add_appender(io: $stdout,
                                       level: config.log_level,
-                                      formatter: config.rails_semantic_logger.format)
+                                      formatter: SafeJsonFormatter.new)
 
   config.semantic_logger.application = if Sidekiq.server?
                                          'vets-api-worker'

lib/semantic_logger/safe_json_formatter.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+class SafeJsonFormatter < SemanticLogger::Formatters::Json
+  # Raw formatter path can call `log.file_name_and_line`, which expects an
+  # Exception-like object with a backtrace. Skip source extraction when callers
+  # pass non-exceptions such as a String to `exception:`.
+  def file_name_and_line
+    return if log.exception && !log.exception.respond_to?(:backtrace)
+
+    super
+  end
+
+  def exception
+    return unless log.exception
+
+    if log.exception.respond_to?(:backtrace)
+      super
+    else
+      hash[:exception] = {
+        name: log.exception.class.name,
+        message: sanitize_string(log.exception.to_s)
+      }
+    end
+  end
+
+  def call(log, logger)
+    super
+  rescue JSON::GeneratorError
+    fallback = hash || {}
+    sanitize_values(fallback).tap do |safe|
+      # String keys: sanitize_values normalizes hash keys to UTF-8 strings for JSON.
+      safe['level'] ||= log.level.to_s
+      safe['message'] ||= '[SafeJsonFormatter: original message unrecoverable]'
+      safe['safe_json_formatter_fallback'] = true
+    end.to_json
+  end
+
+  private
+
+  def sanitize_string(value)
+    value.to_s.encode('UTF-8', invalid: :replace, undef: :replace, replace: '?')
+  end
+
+  def sanitize_values(value)
+    case value
+    when String
+      sanitize_string(value)
+    when Hash
+      value.each_with_object({}) do |(key, nested), acc|
+        acc[sanitize_hash_key(key)] = sanitize_values(nested)
+      end
+    when Array
+      value.map { |nested| sanitize_values(nested) }
+    else
+      value
+    end
+  end
+
+  def sanitize_hash_key(key)
+    sanitize_string(key.to_s)
+  end
+end

spec/lib/semantic_logger/safe_json_formatter_spec.rb

@@ -0,0 +1,130 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe SafeJsonFormatter do
+  subject(:formatter) { described_class.new }
+
+  let(:logger_context) do
+    double(
+      host: 'test-host',
+      application: 'vets-api',
+      environment: 'test'
+    )
+  end
+
+  def build_log(message:, exception: nil, payload: nil)
+    log = SemanticLogger::Log.new('SafeJsonFormatterSpec', :error, 0)
+    log.instance_variable_set(:@message, message)
+    log.instance_variable_set(:@exception, exception)
+    log.instance_variable_set(:@payload, payload)
+    log
+  end
+
+  describe 'FM1 string-as-exception handling' do
+    it 'does not raise when exception is a String' do
+      log = build_log(message: 'upload failed', exception: 'string exception')
+
+      expect { formatter.call(log, logger_context) }.not_to raise_error
+    end
+
+    it 'handles string exception and invalid UTF-8 in payload together' do
+      invalid_bytes = +"\xA1x"
+      invalid_bytes.force_encoding(Encoding::ASCII_8BIT)
+
+      log = build_log(
+        message: 'both failure modes',
+        exception: 'string-as-exception',
+        payload: { body: invalid_bytes }
+      )
+
+      parsed = JSON.parse(formatter.call(log, logger_context))
+
+      expect(parsed.dig('exception', 'name')).to eq('String')
+      expect(parsed.dig('exception', 'message')).to eq('string-as-exception')
+      expect(parsed['safe_json_formatter_fallback']).to be true
+      expect(parsed['payload']['body']).to eq('?x')
+    end
+  end
+
+  describe 'FM2 encoding sanitization' do
+    it 'does not raise JSON::GeneratorError for invalid UTF-8 payload content' do
+      invalid_bytes = +"\xA1bad-data"
+      invalid_bytes.force_encoding(Encoding::ASCII_8BIT)
+
+      log = build_log(
+        message: 'binary payload',
+        payload: {
+          body: invalid_bytes,
+          nested: ['ok', { detail: invalid_bytes }]
+        }
+      )
+
+      expect { formatter.call(log, logger_context) }.not_to raise_error
+    end
+
+    it 'marks logs that used the JSON fallback path' do
+      invalid_bytes = +"\xA1bad-data"
+      invalid_bytes.force_encoding(Encoding::ASCII_8BIT)
+
+      log = build_log(
+        message: 'binary payload',
+        payload: { body: invalid_bytes }
+      )
+
+      parsed = JSON.parse(formatter.call(log, logger_context))
+
+      expect(parsed['safe_json_formatter_fallback']).to be true
+      expect(parsed['payload']['body']).to eq('?bad-data')
+      expect(parsed['level']).to be_present
+      expect(parsed['message']).to eq('binary payload')
+    end
+
+    it 'sanitizes invalid UTF-8 in hash keys so the fallback JSON encodes' do
+      bad_key = +"\xA1meta"
+      bad_key.force_encoding(Encoding::ASCII_8BIT)
+
+      log = build_log(
+        message: 'bad key',
+        payload: { bad_key => 'ok' }
+      )
+
+      parsed = JSON.parse(formatter.call(log, logger_context))
+
+      expect(parsed['safe_json_formatter_fallback']).to be true
+      expect(parsed['payload'].keys).to eq(['?meta'])
+      expect(parsed['payload']['?meta']).to eq('ok')
+    end
+  end
+
+  describe 'standard exception serialization' do
+    it 'continues to serialize a real exception' do
+      error = RuntimeError.new('Exception!')
+      error.set_backtrace(%w[a.rb:1 b.rb:2])
+      log = build_log(message: 'failed', exception: error)
+
+      result = formatter.call(log, logger_context)
+      parsed = JSON.parse(result)
+
+      expect(parsed.dig('exception', 'name')).to eq('RuntimeError')
+      expect(parsed.dig('exception', 'message')).to eq('Exception!')
+      expect(parsed).not_to have_key('safe_json_formatter_fallback')
+    end
+  end
+
+  describe 'JSON fallback metadata' do
+    it 'fills placeholder message when the original is absent after failure' do
+      invalid_bytes = +"\xFF"
+      invalid_bytes.force_encoding(Encoding::ASCII_8BIT)
+
+      log = SemanticLogger::Log.new('SafeJsonFormatterSpec', :error, 0)
+      log.instance_variable_set(:@message, nil)
+      log.instance_variable_set(:@payload, { body: invalid_bytes })
+
+      parsed = JSON.parse(formatter.call(log, logger_context))
+
+      expect(parsed['safe_json_formatter_fallback']).to be true
+      expect(parsed['message']).to eq('[SafeJsonFormatter: original message unrecoverable]')
+    end
+  end
+end