You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Eason09053360 (via GitHub)" <gi...@apache.org> on 2026/09/15 10:28:03 UTC

[PR] Mask the SQLite database path in airflow info --anonymize [airflow]

Eason09053360 opened a new pull request, #73179:
URL: https://github.com/apache/airflow/pull/73179

   ## Why
   
   `airflow info --anonymize` exists so the report can be pasted into a public bug report, but it leaves the SQLite database path completely untouched:
   
   ```
   sql_alchemy_conn  | sqlite:////Users/alice/airflow/airflow.db    # leaked
   base_log_folder   | ${HOME}/airflow/logs                         # masked
   airflow_home      | ${HOME}/airflow                              # masked
   ```
   
   `PiiAnonymizer.process_url` only rewrites the netloc, which is where the credentials live for PostgreSQL and MySQL. A SQLite URL has no netloc — the filesystem path *is* the identifying information — so the method falls straight through to `urlunsplit` and returns its input unchanged, silently. SQLite is the default backend, so this hits exactly the people most likely to trust the flag and paste the output somewhere public.
   
   The neighbouring fields in the same table (`plugins_folder`, `base_log_folder`, `remote_base_log_folder`) already go through `process_path` and mask correctly.
   
   ## What
   
   `airflow-core/src/airflow/cli/commands/info_command.py` — when a URL has no netloc, `process_url` now hands the whole value to `process_path`, giving `sqlite:///${HOME}/airflow/airflow.db` and matching how every other path in the report is treated. The `netloc = None` sentinel is dropped since the new branch returns early.
   
   This also stops `urlunsplit` mangling netloc-less URLs: `sqlite:///relative.db` previously came back as `sqlite:/relative.db`.
   
   Both cases are covered by new entries in the existing `test_should_remove_pii_from_url` parametrize list; URLs with a netloc are unchanged.
   
   Two related gaps are deliberately left alone. `uname` still reports the machine hostname — that is being fixed in #73074. And a netloc-less URL can still carry credentials in its query string (`mssql+pyodbc:///?odbc_connect=...PWD=secret`), which `process_url` has never masked; happy to take that in a follow-up if reviewers would rather see it handled here.
   
   One thing worth a reviewer's opinion: `process_path` does unanchored substring replacement, so on the official Docker image (`$HOME=/home/airflow`, user `airflow`, `AIRFLOW_HOME=/opt/airflow`) the connection string now prints as `sqlite:////opt/${USER}/${USER}.db`. That is the same over-replacement `base_log_folder` and `airflow_home` already show today, so this PR keeps `sql_alchemy_conn` consistent with its neighbours rather than special-casing it. Anchoring the replacement would be a separate change across all five fields.
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — Claude Code (Opus 5)
   
   Generated-by: Claude Code (Opus 5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   


-- 
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


Re: [PR] Mask the SQLite database path in airflow info --anonymize [airflow]

Posted by "Eason09053360 (via GitHub)" <gi...@apache.org>.
Eason09053360 commented on code in PR #73179:
URL: https://github.com/apache/airflow/pull/73179#discussion_r4073384604


##########
airflow-core/src/airflow/cli/commands/info_command.py:
##########
@@ -119,6 +118,10 @@ def process_url(self, value) -> str:
                 netloc = host
             else:
                 netloc = ""
+        else:
+            # A netloc-less URL is a local-file backend (SQLite is the default), where
+            # the path itself is the identifying information.
+            return self.process_path(value)

Review Comment:
   Thanks for reviewing
   Agreed on both — and I'm deliberately not opening a follow-up PR for either but I will find someone solving it! :)
   Thanks



-- 
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


Re: [PR] Mask the SQLite database path in airflow info --anonymize [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #73179:
URL: https://github.com/apache/airflow/pull/73179


-- 
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


Re: [PR] Mask the SQLite database path in airflow info --anonymize [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #73179:
URL: https://github.com/apache/airflow/pull/73179#discussion_r4073003601


##########
airflow-core/src/airflow/cli/commands/info_command.py:
##########
@@ -119,6 +118,10 @@ def process_url(self, value) -> str:
                 netloc = host
             else:
                 netloc = ""
+        else:
+            # A netloc-less URL is a local-file backend (SQLite is the default), where
+            # the path itself is the identifying information.
+            return self.process_path(value)

Review Comment:
   `nit` — the neighbouring fields hand `process_path` a bare filesystem path, but here it also gets the scheme, so the unanchored substitution can chew on the scheme itself:
   
   ```text
   username "lite",  sqlite:////home/lite/airflow.db  ->  sq${USER}:///${HOME}/airflow.db
   username "db",    duckdb:////home/db/wh.duckdb     ->  duck${USER}:///${HOME}/wh.duck${USER}
   ```
   
   Splitting on the first colon keeps the scheme out of reach and is byte-identical on every realistic input (checked against the Docker `sqlite:////opt/${USER}/${USER}.db` case, `sqlite:///:memory:`, `sqlite://` and the `NOT AVAILABLE` fallback):
   
   ```python
   scheme, sep, rest = value.partition(":")
   return f"{scheme}{sep}{self.process_path(rest)}"
   ```
   
   Marginal — netloc-less schemes are `sqlite`, `duckdb` and `mssql+pyodbc`, so it takes a username like `lite` or `db` to bite. Not blocking.



-- 
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