You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "arnoldmr01 (via GitHub)" <gi...@apache.org> on 2025/11/25 05:26:50 UTC
[PR] Bugfix/mask sensitive values in cli [airflow]
arnoldmr01 opened a new pull request, #58659:
URL: https://github.com/apache/airflow/pull/58659
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
related: https://github.com/apache/airflow/issues/55947
## The purpose of this PR:
Mask sensitive values in cli
The sensitive values are:
```
secret_key
jwt_secret
asset_manager_kwargs
fernet_key
sql_alchemy_conn
sql_alchemy_conn_async
sql_alchemy_engine_args
remote_task_handler_kwargs
backend_kwargs
sentry_dsn
secret_key
secrets_backend_kwargs
```
### What this PR changed:
- Add two parameter `hide_sensitive_values`, and `is_sensitive` in `def _write_value`
- Add logic to check if the value is sensitive. If it is, print the value as "< hidden >", if not, print the original value.
- Add new cli flag `--hide-sensitive`, so user can determine whether to hide the sensitive values.
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2633979776
##########
airflow-core/src/airflow/cli/commands/config_command.py:
##########
@@ -48,6 +55,8 @@ def show_config(args):
include_providers=not args.exclude_providers,
comment_out_everything=args.comment_out_everything or args.defaults,
only_defaults=args.defaults,
+ non_sensitive=args.non_sensitive,
+ show_values=args.show_values,
Review Comment:
```suggestion
show_values=show_values,
```
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "NewSmoke38 (via GitHub)" <gi...@apache.org>.
NewSmoke38 commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3698243519
Hi @arnoldmr01, @potiuk, and @jason810496!
I’ve prepared the documentation updates for set-config.rst and the docker-compose.yaml to align them with the new masking logic. Since I can't push or suggest in the main branch as these files aren't in the PR yet, I am putting my changes here:
1. In `set-config.rst`
Line 31:
```
airflow config list --defaults --show-values
```
Line 45:
```
airflow config list --defaults --show-values > "${AIRFLOW_HOME}/airflow.cfg"
```
after this add this note -
```
.. note::
By default, Airflow masks sensitive values (passwords, secrets, connection strings)
in CLI output to prevent accidental exposure. You must include the ``--show-values`` flag when generating or
exporting configuration files.
Running ``airflow config list`` without this flag and redirecting to a file will save non-functional placeholders
(e.g., ``<hidden>``). This will cause Airflow to fail when it attempts to use those configurations.
```
2. In `docker-compose.yaml`
Line 270:
```
/entrypoint airflow config list --show-values >/dev/null
```
I'll search across the entire codebase now to identify any other instances in documentation or scripts where `airflow config list` is used for initialization or export. I'll follow up here if I find more areas that need updating.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3583263202
@potiuk OK, I will modify it.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "NewSmoke38 (via GitHub)" <gi...@apache.org>.
NewSmoke38 commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3700496573
Thanks @potiuk and @jason810496!
Since the documentation and docker-compose.yaml updates are being tracked as a separate task in #59839, I'll go ahead and open a dedicated PR for those changes directly to apache/airflow repo.
I'll make sure to link both issues in the new PR. Thanks for the guidance!
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2647345380
##########
shared/configuration/src/airflow_shared/configuration/parser.py:
##########
@@ -1595,29 +1595,39 @@ def _write_value(
needs_separation: bool,
only_defaults: bool,
section_to_write: str,
+ non_sensitive: bool,
+ is_sensitive: bool,
Review Comment:
Thanks for your advice. I will modify it with `hide-sensitive`
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2583962711
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -789,6 +789,11 @@ def string_lower_type(val):
("option",),
help="The option name",
)
+ARG_CONFIG_HIDE_SENSITIVE = Arg(
+ ("--hide-sensitive",),
+ action="store_true",
+ help="Hide sensitive configuration values (passwords, keys, etc.) in the output.",
+)
Review Comment:
Let's follow by the letter what we agreed. Thsi is not an opinion any more, this is something what we agreed to on devlist.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2646503654
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -790,6 +790,20 @@ def string_lower_type(val):
("option",),
help="The option name",
)
+ARG_NON_SENSITIVE = Arg(
+ ("--non-sensitive",),
+ action="store_true",
+ help="When used with --show-values, hide sensitive values (passwords, keys, tokens, etc.) and only show non-sensitive configuration values.",
+)
+ARG_CONFIG_SHOW_VALUES = Arg(
+ ("--show-values",),
+ action="store_true",
+ help=(
+ "Show configuration values. "
+ "By default only option names are shown and values (including potentially "
+ "sensitive ones) are hidden."
+ ),
+)
Review Comment:
Thanks for sharing the context! I see, then we could keep it as is. 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] Bugfix/mask sensitive values in cli [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2567412187
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -789,6 +789,11 @@ def string_lower_type(val):
("option",),
help="The option name",
)
+ARG_CONFIG_HIDE_SENSITIVE = Arg(
+ ("--hide-sensitive",),
+ action="store_true",
+ help="Hide sensitive configuration values (passwords, keys, etc.) in the output.",
+)
Review Comment:
Should we hide by default and have `--no-mask-sensitive` instead?
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3677559890
Here are the key traceback for the investigation
```
File "/opt/airflow/task-sdk/src/airflow/sdk/api/client.py", line 182, in raise_on_4xx_5xx
return get_json_error(response) or response.raise_for_status()
File "/opt/airflow/task-sdk/src/airflow/sdk/api/client.py", line 178, in get_json_error
raise err
airflow.sdk.api.client.ServerResponseError: Invalid auth token: Signature verification failed
```
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2584136016
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -789,6 +789,11 @@ def string_lower_type(val):
("option",),
help="The option name",
)
+ARG_CONFIG_HIDE_SENSITIVE = Arg(
+ ("--hide-sensitive",),
+ action="store_true",
+ help="Hide sensitive configuration values (passwords, keys, etc.) in the output.",
+)
Review Comment:
> Should we have another flag to show only the non-sensitive values?
I think we can add that one flag.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2646290035
##########
airflow-core/src/airflow/cli/commands/config_command.py:
##########
@@ -48,6 +55,8 @@ def show_config(args):
include_providers=not args.exclude_providers,
comment_out_everything=args.comment_out_everything or args.defaults,
only_defaults=args.defaults,
+ non_sensitive=args.non_sensitive,
+ show_values=args.show_values,
Review Comment:
OK, I will modify it.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3693756950
Nice!
> The final thing is that we need to double check on whether do we need to update the flag usage or new behavior across the the user facing documentations and docker compose.
Good point!
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3704274080
@jason810496 What is OpenLineage test? Do you mean the provider test of openlineage?and how can I run the test? I tried to run `breeze testing providers-tests --parallel-test-types "Providers[openlineage]"` and I got the error in the pic. I'm not sure if I run the test correctly. If I did it wrong please let me know. Thanks for your time.
<img width="1050" height="295" alt="image" src="https://github.com/user-attachments/assets/74348071-35ec-4f3b-b2d0-3f6d16d542b3" />
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2633982164
##########
airflow-core/src/airflow/cli/commands/config_command.py:
##########
@@ -48,6 +55,8 @@ def show_config(args):
include_providers=not args.exclude_providers,
comment_out_everything=args.comment_out_everything or args.defaults,
only_defaults=args.defaults,
+ non_sensitive=args.non_sensitive,
+ show_values=args.show_values,
Review Comment:
if not, then we probably don't need the `getattr` check?
##########
airflow-core/src/airflow/cli/commands/config_command.py:
##########
@@ -37,6 +38,12 @@
@providers_configuration_loaded
def show_config(args):
"""Show current application configuration."""
+ show_values = getattr(args, "show_values", False)
+ if not show_values:
Review Comment:
```suggestion
if not (show_values := getattr(args, "show_values", False)):
```
##########
airflow-core/src/airflow/cli/commands/config_command.py:
##########
@@ -48,6 +55,8 @@ def show_config(args):
include_providers=not args.exclude_providers,
comment_out_everything=args.comment_out_everything or args.defaults,
only_defaults=args.defaults,
+ non_sensitive=args.non_sensitive,
+ show_values=args.show_values,
Review Comment:
```suggestion
show_values=show_values,
```
##########
airflow-core/src/airflow/cli/commands/config_command.py:
##########
@@ -48,6 +55,8 @@ def show_config(args):
include_providers=not args.exclude_providers,
comment_out_everything=args.comment_out_everything or args.defaults,
only_defaults=args.defaults,
+ non_sensitive=args.non_sensitive,
+ show_values=args.show_values,
Review Comment:
Will it be an issue if `args.show_values` does not exist? (since you use getattr to check above)
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2638994760
##########
airflow-core/src/airflow/cli/commands/config_command.py:
##########
@@ -37,6 +38,12 @@
@providers_configuration_loaded
def show_config(args):
"""Show current application configuration."""
+ show_values = getattr(args, "show_values", False)
+ if not show_values:
Review Comment:
I think it’s enoough to do `if not args.show_values` here
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2639169518
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -790,6 +790,20 @@ def string_lower_type(val):
("option",),
help="The option name",
)
+ARG_NON_SENSITIVE = Arg(
+ ("--non-sensitive",),
+ action="store_true",
+ help="When used with --show-values, hide sensitive values (passwords, keys, tokens, etc.) and only show non-sensitive configuration values.",
+)
+ARG_CONFIG_SHOW_VALUES = Arg(
+ ("--show-values",),
+ action="store_true",
+ help=(
+ "Show configuration values. "
+ "By default only option names are shown and values (including potentially "
+ "sensitive ones) are hidden."
+ ),
+)
Review Comment:
How about removing `--show-values` flag to remain the behavior as is (showing the value by default without the `--show-values`) to minimize breaking change to user?
Then we might need to change the `--non-sensitive` somehow like `--show-sensitive` and `--show-sensitive` will be default as False.
##########
shared/configuration/src/airflow_shared/configuration/parser.py:
##########
@@ -1595,29 +1595,39 @@ def _write_value(
needs_separation: bool,
only_defaults: bool,
section_to_write: str,
+ non_sensitive: bool,
+ is_sensitive: bool,
Review Comment:
We might need to rename `non_sensitive` parameter under this context, as we also have `is_sensitive` parameter for the same function, so the purpose of `non_sensitive` might be unclear.
Maybe we could name as `hide_sensitive`, `redact_sensitive` or `show_values` etc.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 merged PR #58659:
URL: https://github.com/apache/airflow/pull/58659
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2583730628
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -789,6 +789,11 @@ def string_lower_type(val):
("option",),
help="The option name",
)
+ARG_CONFIG_HIDE_SENSITIVE = Arg(
+ ("--hide-sensitive",),
+ action="store_true",
+ help="Hide sensitive configuration values (passwords, keys, etc.) in the output.",
+)
Review Comment:
@uranusjr I think that will be a better option. Thus, the situation will be
- CLI`* list` will not show any values
- flag like `--no-mask-sensitive` will not mask the sensitive values and show all the values.
Should we have another flag to show only the non-sensitive values?
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2634572635
##########
airflow-core/src/airflow/cli/commands/config_command.py:
##########
@@ -48,6 +55,8 @@ def show_config(args):
include_providers=not args.exclude_providers,
comment_out_everything=args.comment_out_everything or args.defaults,
only_defaults=args.defaults,
+ non_sensitive=args.non_sensitive,
+ show_values=args.show_values,
Review Comment:
If `args.show_values` does not exit, I think it will cause `AttributeError`. However
`
ARG_CONFIG_SHOW_VALUES = Arg(
("--show-values",),
action="store_true",
help=(
"Show configuration values. "
"By default only option names are shown and values (including potentially "
"sensitive ones) are hidden."
),
)
`
may ensure that `args.show_values` exit.
Under this situation, do you think we can just directly use args.show_values? Or is there any suggesting?
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3704360337
It's more like new feature/ breaking change instead of fix, so I didn't add `backport-to-v3-1-test` before merge. We can manual backport if necessary.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3699083013
Thanks @NewSmoke38 ! Good review/check
> Since I can't push or suggest in the main branch as these files aren't in the PR yet, I am putting my changes here:
Yeah. Only committer's can - but you can also make a PR to this PR and ask @arnoldmr01 to merge it. All you need to do is to `gh co #58659`, make modifications, create your own branch and push to your fork and create a PR to @arnoldmr01 repo branch `arnoldmr01:bugfix/mask-sensitive-values-in-cli`.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2646289668
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -790,6 +790,20 @@ def string_lower_type(val):
("option",),
help="The option name",
)
+ARG_NON_SENSITIVE = Arg(
+ ("--non-sensitive",),
+ action="store_true",
+ help="When used with --show-values, hide sensitive values (passwords, keys, tokens, etc.) and only show non-sensitive configuration values.",
+)
+ARG_CONFIG_SHOW_VALUES = Arg(
+ ("--show-values",),
+ action="store_true",
+ help=(
+ "Show configuration values. "
+ "By default only option names are shown and values (including potentially "
+ "sensitive ones) are hidden."
+ ),
+)
Review Comment:
Thanks for the suggestion. While it does preserve the current behavior, the dev list here (https://lists.apache.org/thread/3dhzqvpw4z2x0wfokpmdncsjwws86zcs
) agreed that * list should return keys only; values should be shown via --show-values.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2646289901
##########
airflow-core/src/airflow/cli/commands/config_command.py:
##########
@@ -37,6 +38,12 @@
@providers_configuration_loaded
def show_config(args):
"""Show current application configuration."""
+ show_values = getattr(args, "show_values", False)
+ if not show_values:
Review Comment:
OK, I will modify it.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3704364499
> What is OpenLineage test? Do you mean the provider test of openlineage?
I mean the previous CI failure https://github.com/apache/airflow/pull/58659#pullrequestreview-3603204213
> and how can I run the test? I tried to run breeze testing providers-tests --parallel-test-types "Providers[openlineage]" and I got the error in the pic. I'm not sure if I run the test correctly. If I did it wrong please let me know. Thanks for your time.
I had added `full tests needed` label then closed and opened PR (which will trigger CI again with the `full tests needed` label I added), so we had already ran the full test on GitHub Action. Anyway, we are all set and we don't need to manual test ourself.
> If I did it wrong please let me know.
It seems like just a flaky test case instead running the command wrong.
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3573834527
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [prek-hooks]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3576645005
This is a good idea but it's not going far enough.
We recently agreed on a different approach https://lists.apache.org/thread/3dhzqvpw4z2x0wfokpmdncsjwws86zcs
Part of this approach was that config list should not show values at all (just list configs) , then I think we should add flag to show non-sensitive values and mask sensitive, and third option should be show all.
Can you adapt please to the latest consensus ?
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2583961037
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -789,6 +789,11 @@ def string_lower_type(val):
("option",),
help="The option name",
)
+ARG_CONFIG_HIDE_SENSITIVE = Arg(
+ ("--hide-sensitive",),
+ action="store_true",
+ help="Hide sensitive configuration values (passwords, keys, etc.) in the output.",
+)
Review Comment:
We agreed on the devlist (and there was a lazy consensus on that) - see https://lists.apache.org/thread/3dhzqvpw4z2x0wfokpmdncsjwws86zcs point 5:
> 5) local CLI * list (connections, variables, config) only by default
returns "keys" - and it will only return values when `--show-values`
is passed as command line option (with clear comment in help that this
option **might** show sensitive data, also when we do `* list` command
without `--show-values` we emit stderr output explaining that
potentially sensitive data is hidden and you need to specify
`--show-values` to see them
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on PR #58659:
URL: https://github.com/apache/airflow/pull/58659#issuecomment-3584452687
@potiuk With my comprehension, CLI `* list` should not show value. There should be two situations 1) `--non-sensitive` to only show the non-sensitive value. 2) `--show-all` to show all values. Am I right?
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #58659:
URL: https://github.com/apache/airflow/pull/58659#discussion_r2634572635
##########
airflow-core/src/airflow/cli/commands/config_command.py:
##########
@@ -48,6 +55,8 @@ def show_config(args):
include_providers=not args.exclude_providers,
comment_out_everything=args.comment_out_everything or args.defaults,
only_defaults=args.defaults,
+ non_sensitive=args.non_sensitive,
+ show_values=args.show_values,
Review Comment:
If `args.show_values` does not exit, I think it will cause `AttributeError`. However
`
ARG_CONFIG_SHOW_VALUES = Arg(
("--show-values",),
action="store_true",
help=(
"Show configuration values. "
"By default only option names are shown and values (including potentially "
"sensitive ones) are hidden."
),
)
`
may ensure that `args.show_values` exit.
In this situation, do you think we can just directly use args.show_values? Or is there any suggesting?
--
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] Bugfix/mask sensitive values in cli [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 closed pull request #58659: Bugfix/mask sensitive values in cli
URL: https://github.com/apache/airflow/pull/58659
--
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