ashb commented on issue #16007: URL: https://github.com/apache/airflow/issues/16007#issuecomment-847937657
@potiuk The guarding I am thinking of is in Connection, not in the masker (The top if here is in the `__init__` function): https://github.com/apache/airflow/blob/9c98a60cdd29f0b005bf3abdbfc42aba419fded8/airflow/models/connection.py#L145-L151 The attack scenario you mentioned was also bought up by @uranusjr here https://github.com/apache/airflow/pull/15599#issuecomment-830104043 and my reply > Possibly, but I don't think we need to defend against that level of attack -- it would only show up in the UI like that if it matches a connection the task has accessed. > > So yes, there's a theoretical attack surface here, but with the planned work of per-Connection ACLs etc, I think that is mitigated. Plus it only lets you validate the password if you already know it, but if you can change the DAG code, there are easier ways of exfil-ing the credentials. How about this: Currently we somewhat "blindly" replace in the rendered output, but I think it is also possible that we could operate on the format strings. Consider the following case ``` conn = Connection.from_uri("foo://a:b@host") logging.info("Password: %s", conn.password) logging.info("Password: " + conn.password) ``` The first log line we could, I think _always_ replace the password since it's a whole string, but replacing in a longer message only could be done only when the password length is >=4 for example. (I think this is achievable on a technical level). That said I'm not sure _how_ they ended up with every character being masked. -- 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. For queries about this service, please contact Infrastructure at: [email protected]
