Repository navigation
Mask extra and credentials in connections list --hide-sensitive - #73157
Eason09053360 wants to merge 1 commit into
Conversation
The flag's help promises to mask passwords, URI credentials and extra, so its output is safe to paste into a bug report or a log. Two cases broke that promise in the `get_uri` column, while the neighbouring `password` and `extra_dejson` columns masked correctly — which makes the leak easy to miss when eyeballing the output. `Connection.get_uri` serialises `extra` into the query string, and a `host` that carries its own scheme makes it emit a doubled scheme that `urlsplit` parses with the credentials left outside `netloc`.
|
Hello @Eason09053360 - thank you for your contributions to Apache Airflow! The Airflow community has introduced a limit of 5 open pull requests at a time for contributors without write access to the repository. You currently have 33 open pull requests, so - as a one-time step of introducing the limit - we closed the ones where maintainers have not engaged yet:
These pull requests stay open because maintainers are already engaged in them - they count towards your limit:
This is not a judgement of you or of your changes. We never told contributors before that opening many pull requests at once was a problem, so there is nothing to feel bad about - and nothing is lost: your branches, commits and the review history stay where they are. What we ask you to do is to make your first prioritization decision: choose which of the pull requests above matter most to you, and reopen them (up to 5 open at a time, including the ones still open) with the "Reopen pull request" button or While your pull requests are waiting for review, the most valuable thing you can do is help in other ways - reviewing other contributors' pull requests, helping with issues, and taking part in the discussions on the devlist and Slack. Why we introduced the limit, what it means for you and how to reopen or restore a pull request is explained in https://github.com/apache/airflow/blob/main/contributing-docs/32_open_pull_request_limit.rst. Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting |
Why
airflow connections list --show-values --hide-sensitiveexists so its output can be pasted into a bug report or kept in a log, and the flag's help promises to mask "passwords, URI credentials, extra". Theget_uricolumn broke that promise in two ways, while the neighbouringpasswordandextra_dejsoncolumns masked correctly — so the leak is easy to miss when eyeballing the output.extrawas printed verbatim.Connection.get_uri()serialisesextrainto the query string, and the masking helper only ever looked at credentials innetloc:The password leaked too when
hostcarries its own scheme.get_uri()then emits a doubled scheme, whichurlsplitparses as netlochttps:with the credentials left inpath, so the"@" in netlocguard never fired:related: #59838
What
_mask_uri_credentialsinairflow-core/src/airflow/cli/commands/connection_command.pynow masks the query string as a whole — matching how theextra_dejsoncolumn in the same mapper treatsextra— and takes the authority from the last://instead of trustingparsed.netloc.Both spans are substituted in the original string rather than reassembled with
urlunsplit, which mangles URIs with an empty authority (filesystem://→filesystem:,http:///?…→http:/?…) — reachable here because a connection can haveextrawithout a host.Output for the four connection shapes:
Connections with nothing to mask (
redis://localhost:6379/0,sqlite:///tmp/test.db) are returned byte-for-byte unchanged.Tests in
airflow-core/tests/unit/cli/commands/test_connection_command.pycover both leaks, including two cases that build a realConnectionand assert the secret is absent from_mask_uri_credentials(conn.get_uri()). All eight new cases fail without the change.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5) following the guidelines