potiuk opened a new pull request, #70884: URL: https://github.com/apache/airflow/pull/70884
Follow-up to [#70736](https://github.com/apache/airflow/pull/70736), which merged with a refusal rule that is broader than it needs to be. That change made the team agnostic lookup refuse any id of the form `_<x>___<y>`, on the grounds that such an id could name a team's namespace. That is true of *some* of them, not all: team names are validated as `^[a-zA-Z0-9_-]{3,50}$`, so a one- or two-character leading segment cannot be a team. A team agnostic secret whose id merely looks namespaced — `_a___b`, `_ab___x` — is currently refused even though no team namespace can be spelled that way, which blocks a legitimate lookup for no benefit. ### Approach Test the **leading segment against the team name rule** rather than testing whether the id merely contains the separator: ```python for i in ...: # every position of the separator team, rest = secret_id[1:i], secret_id[i + 3:] if rest and _TEAM_NAME.fullmatch(team): return True ``` Every id that does spell a real team's namespace still has a valid team name in that position by construction, so nothing reachable is let through. Ids that cannot name a team resolve normally again. Every split is considered, because a team name may itself contain the separator — `_a___b___c` is both team `a` with id `b___c` and team `a___b` with id `c` — and one plausible team name is enough to refuse. Overlapping separator positions are included, so `_abc____x` is tested at both offsets. **`teams sync` now enforces the team name rule.** This is what makes the above sound: the guard assumes a stored team name is a valid one. `airflow teams sync` creates teams from the dag bundle config and did not validate the names at all, unlike `teams create` — so an unvalidated short name such as `a` would make the test miss and reopen the cross-team read that #70736 closed. The pattern is factored into `TEAM_NAME_PATTERN` and applied on both creation paths, with a comment on the sync path recording why the secrets backend depends on it. ### Behaviour change - Ids whose leading segment cannot be a team name resolve through the team agnostic lookup again, for every caller scope. Ids that could name a team stay refused exactly as #70736 made them. - `airflow teams sync` now fails with a clear message when the dag bundle config contains a team name that does not match `^[a-zA-Z0-9_-]{3,50}$`, where it previously created the team silently. ### Test plan - [x] `test_secrets_environment_variables.py` — 46 cases pass - [x] New `test_id_that_cannot_name_a_team_is_still_resolved` — `_a___b` and `_ab___x` resolve for every caller scope including none; it fails against the currently merged rule - [x] The existing cross-team tests are unchanged and still pass, including `test_team_whose_name_extends_the_callers_is_not_readable` - [x] `test_team_command.py` — 21 cases pass with the shared pattern - [x] Exhaustive property check over six team names chosen to collide (`team_a` / `team_a___prod`, `abc` / `abc___b`), both lookups, every caller scope, bare and namespaced ids: **0 cross-team resolutions** - [x] `ruff check` / `ruff format` clean ##### 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]
