potiuk opened a new pull request, #70755:
URL: https://github.com/apache/airflow/pull/70755
Options are registered as sensitive under their **base** section, but a team
scoped
override lives elsewhere:
- config file — a `[<team>=<section>]` section
- environment — `AIRFLOW__<TEAM>___<SECTION>__<KEY>`
Every sensitivity decision was a direct membership test against the
registered base
pairs, and the masking pass iterated those pairs and looked each section up
verbatim. A team scoped section was therefore never visited, and its value
was
returned in clear text while the same option in the base section was
correctly
hidden.
The environment spelling misses by a further step: `_include_envs` splits
`AIRFLOW__TEAM_A___CELERY__BROKER_URL` on `__` with `maxsplit=2`, yielding
section
`team_a` and key `_celery__broker_url` — a pair that is neither the base nor
the
team scoped section name, so no base keyed loop could reach it either.
### Approach
Sensitivity decisions now resolve the team scoped spelling back to the base
option.
- `team_section_name()` builds the config file section name, and is used at
**both**
construction sites so the two representations cannot drift
- `base_section_name()` recovers the base section from it
- `is_sensitive_option()` tests a pair directly, then via the base section,
then via
the tail an environment variable contributes — so it can only ever
recognise
*more* options as sensitive, never fewer
Two deliberate choices:
**The team name is never parsed out of an environment variable name.** A
team name
may contain underscores, so splitting `AIRFLOW__<TEAM>___<SECTION>__<KEY>` is
ambiguous. The name is matched against the tail each registered sensitive
option
contributes instead. No section ends with `_` or `.` and no key starts with
`_`
anywhere in core or providers, so this cannot collide with a global variable.
**The config file section is split on the *last* separator.** Base section
names
never contain `=` (checked across all sections in `config.yml` and every
`config:`
block in every `provider.yaml`), so the base section is recovered even for a
team
name that contains the separator itself. `airflow teams sync` creates teams
from
dag bundle config, whose `team_name` field is not validated against the CLI's
charset, so such a name is conceivable.
`routes/public/config.py` needed its own two line change: `get_config_value`
never
goes through `as_dict`, so the parser change cannot reach it.
`_get_cmd_option_from_config_sources` and
`_get_secret_option_from_config_sources`
are intentionally left alone. They are only called with base pairs, and
making them
team aware would mean executing team scoped commands and fetching team
secrets,
which the parser deliberately refuses. Those fallbacks are hidden instead.
### Behaviour changes
1. Team scoped values of sensitive options now read `< hidden >` in
`as_dict`,
`GET /config`, `GET /config/section/{section}/option/{option}` and
`write(hide_sensitive=True)` — symmetric with how the base option already
behaved. Anyone using the API to read a team's real value will now get
the mask.
2. Team scoped `_cmd` / `_secret` entries are replaced with `< hidden >` in
place
rather than resolved and deleted as in a base section, since resolution
is not
supported for a team. The command string itself is no longer shown.
3. `write()` masking now covers team sections, so `airflow config list` is
included.
4. Additive public API on the shared parser; no signature changes.
5. No change for non team configuration, and `display_sensitive=True` still
returns
real values.
`as_dict` cost 0.62 ms versus 0.48 ms baseline at 750 options with 40
sensitive
values.
### Test plan
- [x] `shared/configuration/tests/configuration/test_parser.py` — 6 tests:
`base_section_name` parametrized including a separator inside the team name;
`is_sensitive_option` asserting spoofing fails in both directions; `as_dict`
for a file backed team section, with `display_source` and
`display_sensitive=True` variants; team `_cmd`/`_secret` fallbacks; team
environment variable; `write()`
- [x] 68/68 pass in that module; the behavioural ones fail against the
unmodified parser
- [x]
`airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_config.py` —
`TestTeamScopedOptionMasking`, covering the single option path masked and a non
sensitive team scoped option still readable
- [x] `ruff` check and format clean; `mypy` reports the same single
pre-existing error as baseline
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Opus 5 (1M context)
Generated-by: Claude Opus 5 (1M context) following the guidelines at
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
