Resolve team scoped spelling of per-key secrets-backend-kwarg options as sensitive - #71160
Conversation
89cc771 to
94f820e
Compare
… as sensitive
_is_per_key_sensitive_option and _mask_per_key_sensitive_options matched
literal section names ('secrets', 'workers') only, so a team scoped
spelling of the same option -- the [<team>=secrets] config-file section,
or the AIRFLOW__<TEAM>___SECRETS__BACKEND_KWARG__* env var, both of which
are reported under a section named after the team -- was never recognised
as sensitive.
Resolve the section to its base section via base_section_name (the same
helper AirflowConfigParser.is_sensitive_option uses for registered options,
introduced in apache#70755) before deciding sensitivity, in both
_is_per_key_sensitive_option and _mask_per_key_sensitive_options.
closes apache#71037
94f820e to
00fc794
Compare
… as sensitive
_is_per_key_sensitive_option and _mask_per_key_sensitive_options matched
literal section names ('secrets', 'workers') only, so a team scoped
spelling of the same option -- the [<team>=secrets] config-file section,
or the AIRFLOW__<TEAM>___SECRETS__BACKEND_KWARG__* env var, both of which
are reported under a section named after the team -- was never recognised
as sensitive.
Resolve the section to its base section via base_section_name (the same
helper AirflowConfigParser.is_sensitive_option uses for registered options,
introduced in apache#70755) before deciding sensitivity, in both
_is_per_key_sensitive_option and _mask_per_key_sensitive_options.
closes apache#71037
…nsitivity' into fix-71037-team-scoped-secrets-sensitivity
|
This is ready for review — all CI checks are passing. It follows the same base_section_name resolution pattern that #70755 introduced for registered sensitive options, applied here to the synthetic per-key secrets-backend-kwarg options. Happy to address any feedback. |
|
|
|
Honestly the entire additional processing feels very hacky to me and easy to get wrong. |
Move _PER_KEY_SENSITIVE_PREFIXES logic into AirflowConfigParser.is_sensitive_option() so both the config API and the CLI (config list) automatically mask team-scoped per-key secrets-backend-kwarg options, without duplicate masking logic. Addresses review feedback from uranusjr on PR apache#71160.
|
Thanks for the feedback — I've refactored this to address both points. Centralization (comment 2): Moved the per-key sensitive-option detection out of the API service layer entirely and into AirflowConfigParser.is_sensitive_option() itself, alongside the existing team-scoped resolution logic it already has. The _PER_KEY_SENSITIVE_PREFIXES dict and the two standalone helper functions (_is_per_key_sensitive_option, _mask_per_key_sensitive_options) that lived in services/public/config.py are gone — there's now a single source of truth for what counts as sensitive, living where the rest of the sensitivity logic already lives. CLI gap (comment 1): This fell out for free once masking moved into is_sensitive_option() — airflow config list already goes through conf.as_dict() / conf.is_sensitive_option(), so it now masks per-key secrets-backend-kwarg options (both the base spelling and the team-scoped one) with zero changes to config_command.py. Added test_cli_show_config_should_not_show_per_key_secrets_backend_kwargs and test_cli_show_config_should_not_show_team_scoped_per_key_secrets_backend_kwargs to cover it. Both the /config API endpoint and config list CLI command now share the exact same masking path, so there's no risk of the two drifting apart again. All existing + new tests pass (API, CLI, and the shared parser test suite), and prek is clean. |
closes #71037
What
_is_per_key_sensitive_optionand_mask_per_key_sensitive_optionsinairflow-core/src/airflow/api_fastapi/core_api/services/public/config.pymatched literal section names (
secrets,workers) only. A team scopedspelling of the same option -- the
[<team>=secrets]config-file section,or the
AIRFLOW__<TEAM>___SECRETS__BACKEND_KWARG__*environment variable,both of which are reported under a section named after the team -- was
never recognised as sensitive.
Not exploitable today, since
_get_custom_secret_backendis not team-awareyet. But the gap becomes live the moment secrets backends gain team scoping,
and at that point it fails open: a team's backend credentials would be
returned in full by
GET /config.How
Both functions now resolve the section to its base section via
base_section_name-- the same helperAirflowConfigParser.is_sensitive_optionalready uses for registered sensitive options (introduced in #70755) --
before deciding whether the option is sensitive.
_is_per_key_sensitive_option: looks up the prefix underbase_section_name(section)instead ofsection._mask_per_key_sensitive_options: now iterates every section actuallypresent in
conf_dict(rather than only the literalsecrets/workerskeys) and resolves each one to its base section before matching. This
also covers the team scoped env var case, since
conf.as_dictalreadyreports it under the team-derived section.
Testing
Added
TestTeamScopedPerKeyBackendKwargMaskinginairflow-core/tests/unit/api_fastapi/core_api/routes/public/test_config.py,covering:
GET /configredacts a team scoped per-key option under both thesecretsandworkersteam scoped sections, while a non-sensitiveoption in the same response stays untouched.
GET /config/section/{section}/option/{option}redacts a team scopedper-key option the same way it redacts the non-team-scoped one.
Full
test_config.pysuite run clean: 3377 passed, 0 failed.