potiuk commented on code in PR #73074:
URL: https://github.com/apache/airflow/pull/73074#discussion_r4308139252
##########
airflow-core/src/airflow/cli/commands/info_command.py:
##########
@@ -252,6 +260,7 @@ def _system_info(self):
operating_system = OperatingSystem.get_current()
arch = Architecture.get_current()
uname = platform.uname()
+ uname =
uname._replace(node=self.anonymizer.process_hostname(uname.node))
Review Comment:
This replaces the hostname only in the `uname` node field. It still leaks
through the database URL's host
(`postgresql://u:[email protected]/airflow`, which `process_url` leaves
alone) and through any path that contains it (`process_path` only replaces HOME
and USER). Adding `socket.gethostname(): "${HOSTNAME}"` to
`PiiAnonymizer._path_replacements`, and applying the same replacements in
`process_url`, would scrub it everywhere and cover this line too, without the
new `process_hostname()` method.
##########
airflow-core/src/airflow/cli/commands/info_command.py:
##########
@@ -50,14 +50,17 @@ def process_username(self, value) -> str:
def process_url(self, value) -> str:
"""Remove pii from URL."""
+ def process_hostname(self, value) -> str:
+ """Remove pii from hostname."""
Review Comment:
Adding a required method to the `Anonymizer` protocol breaks any outside
implementation with only the three existing methods: `_system_info` now calls
`process_hostname()` unconditionally and raises `AttributeError`. Nothing in
the repo is affected, but the replacement-based approach above avoids the
protocol change 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
