Do not log the hook password even at DEBUG level #22627
Conversation
boring-cyborg
bot
added
the
area:core-operators
github-actions
bot
added
the
full tests needed
|
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
kaxil
force-pushed
the
dont-log-hook-password
branch
from
13b430b to
83ef01b
Compare
|
|
I believe there are some edge ceses where it is not masked (for example when the Hook is called by Bash), and I think the whole idea was to catch those edge cases. |
|
The way That said, I'm thinking we should just remove this DEBUG log in its entirety. We already INFO the conn_id, which should be sufficient for debugging purposes imo. |
Fine for me. |
|
I'll edit the PR to remove it entirely. |
jedcunningham
force-pushed
the
dont-log-hook-password
branch
from
4835d90 to
ff63d9c
Compare
) (cherry picked from commit 88165b3)
) (cherry picked from commit 88165b3)
) (cherry picked from commit 88165b3)
The BaseHook currently logs connection details including password at the DEBUG level. While the password is redacted under normal conditions in task logs, there are edge cases where this can lead to a password leaking into logs, such as calling python code that uses a hook from a BashOperator.
The value of logging the password simply seems small relative to the consequences of leaking to logs even in edge cases. There remain plenty of ways to log the password if that is explicitly what you want to do, such as
airflow connection list.