Disable logging args in ShopifyApp jobs #1086
Conversation
tanema
force-pushed
the
disable_logging_args_in_jobs
branch
from
80e3e5b to
672bbf5
Compare
| private | ||
|
|
||
| def args_info(job) | ||
| log_disabled_classes = %w(ShopifyApp::ScripttagsManagerJob ShopifyApp::WebhooksManagerJob) |
There was a problem hiding this comment.
Is there a way to define this behaviour in the classes themselves? It seems like it would be easy to forget to add new classes to the list.
There was a problem hiding this comment.
maybe we could add an attribute to check on each class but these jobs have been consistently alone for 5 years now :D
There was a problem hiding this comment.
Fair enough 😄 , I just thought that this isn't something we want to forget in the future.
tanema
force-pushed
the
disable_logging_args_in_jobs
branch
from
672bbf5 to
9457598
Compare
| Security | ||
| -------- | ||
|
|
||
| Please be certain to redact any private information from your logs or code snippets such as Api Keys, Api Secrets, and any authentication tokens such as shop_tokens. |
There was a problem hiding this comment.
| Please be certain to redact any private information from your logs or code snippets such as Api Keys, Api Secrets, and any authentication tokens such as shop_tokens. | |
| Please be certain to redact any private information from your logs or code snippets such as API keys, API secrets, and any authentication tokens such as shop tokens. |
There was a problem hiding this comment.
I specifically used shop_token so that they would see that visually and look in their logs for it visually.
There was a problem hiding this comment.
Maybe write it as "shop_tokens"? (bit clunky, I know)
| @@ -21,5 +31,16 @@ class Engine < Rails::Engine | |||
| app.config.middleware.insert_after(ShopifyApp::SameSiteCookieMiddleware, ShopifyApp::JWTMiddleware) | |||
| end | |||
| end | |||
|
|
|||
| initializer "shopify_app.redact_job_params" do | |||
| ActiveSupport.on_load(:active_job) do | |||
There was a problem hiding this comment.
Do we need to consider older Rails apps that don't use ActiveJob?
There was a problem hiding this comment.
ActiveJob was introduced in Rails 4.2 and this gem required 5.2
redacting job params in the shopify app jobs so that sensitive information is not accidentally pasted into issues.
tanema
force-pushed
the
disable_logging_args_in_jobs
branch
from
a2098bf to
2837a78
Compare
|
Hi @andyw8 , Do you know which version of Rails is required for |
Since shop tokens are passed into the parameters of the jobs and there has been a tendency for people to copy and paste these logs while debugging, it seems safest to redact the arguments from the jobs so that this will not happen accidentally in the future.
This will patch ActiveJob::Logging::LogSubscriber for rails < 6.1 and use
log_arguments = falsefor rails >= 6.1