You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/03/30 18:21:03 UTC
[GitHub] [airflow] collinmcnulty opened a new pull request #22627: Do not log the hook password even at DEBUG level
collinmcnulty opened a new pull request #22627:
URL: https://github.com/apache/airflow/pull/22627
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`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22627: Do not log the hook password even at DEBUG level
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22627:
URL: https://github.com/apache/airflow/pull/22627#issuecomment-1083553065
> `extra` does get masked based on key.
I believe there are some edge ceses where it is not mased (for example when the Hook is called by Bash), and I think the whole idea was to catch those edge cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham edited a comment on pull request #22627: Do not log the hook password even at DEBUG level
Posted by GitBox <gi...@apache.org>.
jedcunningham edited a comment on pull request #22627:
URL: https://github.com/apache/airflow/pull/22627#issuecomment-1083561752
The way `extra` is masked based on key covers the edge cases I'm aware of (in this case, MASK_SECRETS_IN_LOGS being False). There easily could be others though 🤷♂️.
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #22627: Do not log the hook password even at DEBUG level
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #22627:
URL: https://github.com/apache/airflow/pull/22627#issuecomment-1083499355
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] collinmcnulty commented on pull request #22627: Do not log the hook password even at DEBUG level
Posted by GitBox <gi...@apache.org>.
collinmcnulty commented on pull request #22627:
URL: https://github.com/apache/airflow/pull/22627#issuecomment-1083589295
I'll edit the PR to remove it entirely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham closed pull request #22627: Do not log the hook password even at DEBUG level
Posted by GitBox <gi...@apache.org>.
jedcunningham closed pull request #22627:
URL: https://github.com/apache/airflow/pull/22627
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on pull request #22627: Do not log the hook password even at DEBUG level
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #22627:
URL: https://github.com/apache/airflow/pull/22627#issuecomment-1083561752
The way `extra` is masked based on key covers the edge cases I'm aware of (in this case, MASK_SECRETS_IN_LOGS being False).
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22627: Do not log the hook password even at DEBUG level
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22627:
URL: https://github.com/apache/airflow/pull/22627#issuecomment-1083581535
> The way `extra` is masked based on key covers the edge cases I'm aware of (in this case, MASK_SECRETS_IN_LOGS being False). There easily could be others though 🤷♂️.
>
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham merged pull request #22627: Do not log the hook password even at DEBUG level
Posted by GitBox <gi...@apache.org>.
jedcunningham merged pull request #22627:
URL: https://github.com/apache/airflow/pull/22627
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #22627: Do not log the hook password even at DEBUG level
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #22627:
URL: https://github.com/apache/airflow/pull/22627#issuecomment-1083553065
> `extra` does get masked based on key.
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on pull request #22627: Do not log the hook password even at DEBUG level
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #22627:
URL: https://github.com/apache/airflow/pull/22627#issuecomment-1083549504
`extra` does get masked based on key.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org