-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
Fix password masking in CLI action_logging #15143
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth adding tests do you think?
@ashb yep, I agree it's worth adding test(s). I was thinking about it when I was preparing the PR, but still trying figure out how to do it in the most proper way. |
Ha, I think the adding more cases to |
Currently as long as argument '-p' if present, code tries to mask it. However, '-p' may mean something else (not password), like a boolean flag. Such cases may result in exception
Added in 6b1f641 |
Currently as long as argument '-p' if present, code tries to mask it. However, '-p' may mean something else (not password), like a boolean flag. Such cases may result in exception (cherry picked from commit 486b764)
closes: #15131
Currently as long as argument '-p' if present, code tries to mask it.
However, '-p' may mean something else (not password), like a boolean flag. Such cases may result in exception.
More detailed analysis can be found in issue #15131 (comment)
The solution here is to only try masking the next arg when we are sure
-p
means "password".^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.