stephen-bracken commented on code in PR #62180:
URL: https://github.com/apache/airflow/pull/62180#discussion_r2846103353
##########
airflow-core/src/airflow/models/connection.py:
##########
@@ -224,9 +226,16 @@ def _normalize_conn_type(conn_type):
return conn_type
def _parse_from_uri(self, uri: str):
+ uri_match = RE_SAFE_LOG_URI.search(uri)
+ if uri_match:
+ # Create sanitised uri for logging
+ pwd = uri_match.group(2)
+ safe_log_uri = uri.replace(pwd, "******")
+ else: # Assume no password in URI
+ safe_log_uri = uri
Review Comment:
Some thoughts:
The secret masker won't work on malformed, unparsed URI strings, as it
relies on the password field being identified which is the purpose of the
`_parse_from_uri` method, so when this error is raised it is not possible to
identify which part of the URI could be the password for all cases. This regex
is a best effort attempt to mask the password in the resulting parse error
where the password component is correctly formed. We can use the secrets masker
on the match from the regex pattern, which would conform to how it is used
elsewhere to mask the password field, but we still need to use the regex or
some other mechanism to identify the password in the URI. `urlsplit` will not
do this for malformed URIs.
An alternative approach is to change this error to only output the connetion
id when the URI is invalid, e.g. `invalid URI for connection my_connection_id`.
This would prevent the password component of the URI from leaking, and tells
the dag developer which connection is malformed so they can check what is
happening to it. We could also enable outputting of the URI only in debug mode
if that is preferable, but my view is that we should make a best effort attempt
to hide passwords where they can be identified, even when the URIs are invalid.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
