Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UpdatesController::LogSubscriber should obfuscate sensitive information #239

Open
florianfelsing opened this issue Jul 20, 2024 · 5 comments

Comments

@florianfelsing
Copy link

florianfelsing commented Jul 20, 2024

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.

@printercu
Copy link
Member

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.

@florianfelsing
Copy link
Author

florianfelsing commented Jul 22, 2024

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
Copy link
Author

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.

@SummitCollie
Copy link

@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

@florianfelsing
Copy link
Author

Thanks for following up with this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants