telegram-bot icon indicating copy to clipboard operation
telegram-bot copied to clipboard

UpdatesController::LogSubscriber should obfuscate sensitive information

Open florianfelsing opened this issue 1 year ago • 5 comments

Right now the gem is basically logging the complete payload:

def start_processing(event)
  info do
    payload = event.payload
    "Processing by #{payload[:controller]}##{payload[:action]}\n" \
    "  Update: #{payload[:update].to_json}"
  end
end

I think that especially in production settings it would be a good practice to at least obfuscate the text parts. As a default or via configuration.

For now I've monkey patched this in my app, but I think this would be a good thing to implement on the gem level? I'd be happy to help implement this.

florianfelsing avatar Jul 20 '24 06:07 florianfelsing

Could you share your patch?

It can be tricky to have some generic solution: somebody may want to log messages with commands (/cmd some text) others may want text of all messages because they don't have any sensitive information.

printercu avatar Jul 21 '24 23:07 printercu

Sure:

module Telegram
  module Bot
    class UpdatesController
      class LogSubscriber
        FILTERED_PARAMS = %i[text].freeze

        def start_processing(event)
          info do
            payload = event.payload
            update = sanitize_sensitive_data(payload[:update])
            "Processing by #{payload[:controller]}##{payload[:action]}\n  " \
              "Update: #{update.to_json}"
          end
        end

        private

        def sanitize_sensitive_data(update)
          parameter_filter.filter(update)
        end

        def parameter_filter
          @parameter_filter ||= ActiveSupport::ParameterFilter.new(FILTERED_PARAMS)
        end
      end
    end
  end
end

Maybe we could also leave the default as it is but provide a config option to enable filtering in logs?

florianfelsing avatar Jul 22 '24 08:07 florianfelsing

Let me know if that makes sense to you / if you have any preferences regarding implementation and I'd be glad to work on this one some time during the week @printercu.

florianfelsing avatar Jul 23 '24 17:07 florianfelsing

@florianfelsing Thanks for the patch! I altered your start_processing implementation so it works with the telegram-bot-types gem (for projects where that's enabled).

Also added a conditional so it won't filter anything in local dev environments.

def start_processing(event)
  info do
    payload = event.payload
    update = payload[:update].to_h
    update = sanitize_sensitive_data(update) unless Rails.env.local?
    "Processing by #{payload[:controller]}##{payload[:action]}\n  " \
      "Update: #{update.to_json}"
  end
end

SummitCollie avatar Nov 01 '24 01:11 SummitCollie

Thanks for following up with this!

florianfelsing avatar Nov 01 '24 15:11 florianfelsing