Skip to content

Reject team names that differ only in case - #70885

Open
potiuk wants to merge 2 commits into
apache:mainfrom
potiuk:reject-team-names-colliding-on-case
Open

Reject team names that differ only in case#70885
potiuk wants to merge 2 commits into
apache:mainfrom
potiuk:reject-team-names-colliding-on-case

Conversation

@potiuk

@potiuk potiuk commented Aug 1, 2026

Copy link
Copy Markdown
Member

Stacked on #70884 — both
touch team_command.py. Review that one first; this PR's own change is the
second commit.

Team names are case sensitive — data_eng and Data_Eng are two distinct teams,
both accepted by ^[a-zA-Z0-9_-]{3,50}$ — but the environment secrets backend
upper-cases the team name when it builds the variable name:

os.environ.get(f"{CONN_ENV_PREFIX}_{team_name.upper()}___" + conn_id.upper())

So both resolve AIRFLOW_CONN__DATA_ENG___<ID>. The two teams share one secrets
namespace: each reads the other's Connections and Variables, and whichever
provisions a variable last wins.

Whether the two rows can coexist at all is decided by the database's collation
on a String primary key rather than by Airflow, so the same configuration
behaves differently across backends — on a case sensitive collation both persist
and the namespaces merge; on a case insensitive one the second teams create
fails on the primary key.

Approach

Reject the collision at creation, on both paths.

  • teams create refuses a name that upper-cases onto an existing team's name.
  • teams sync checks the incoming bundle names against each other as well as
    against the stored ones, because one bundle config can introduce both halves of
    a collision in the same run.

The alternative — dropping the upper() fold so team identity stays case
sensitive end to end — was rejected: environment variable names are conventionally
upper-case, and it would silently change which variable an existing deployment
resolves. Failing at the moment the name is chosen is the safer half of that
trade, and it makes behaviour uniform across database collations.

upper() rather than casefold() is deliberate: it is exactly what the backend
applies, and team names are ASCII-only by the name pattern.

Behaviour change

airflow teams create and airflow teams sync now fail with an explanatory
message when a name differs from an existing team's only by case. Deployments that
already have such a pair are not migrated by this change — the collision
already exists there and is reported by neither command. Names differing by more
than case are unaffected.

Test plan

  • test_team_command.py — 23 cases pass
  • test_team_create_rejects_a_name_differing_only_in_case — creating data_eng
    then Data_Eng fails, and no second row is written
  • test_team_create_allows_a_name_differing_by_more_than_casedata_eng and
    data_eng2 coexist
  • ruff check / ruff format clean
Was generative AI tooling used to co-author this PR?
  • 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

potiuk added 2 commits August 1, 2026 03:16
Refusing every id of the form `_<x>___<y>` was too broad. A team agnostic
secret whose id merely looks namespaced -- `_a___b`, `_ab___x` -- was refused
even though `a` and `ab` are too short to be team names, so no team namespace
can be spelled that way and the lookup was safe. That blocked a legitimate
lookup for no benefit.

Test the leading segment against the team name rule instead. 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, while ids that cannot name a
team resolve normally again. Every split is considered, since a team name may
itself contain the separator, and one plausible team name is enough to refuse.

This makes the guard depend on stored team names actually being valid, so
`teams sync` now enforces the same rule as `teams create`. It creates teams
from the dag bundle config and did not validate the names at all, which would
have left the guard's assumption unbacked -- an unvalidated short name such as
"a" would make the test miss and reopen the cross-team read.

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
Team names are case sensitive -- `data_eng` and `Data_Eng` are two distinct
teams -- but the environment secrets backend upper-cases the team name when it
builds the variable name, so both resolve `AIRFLOW_CONN__DATA_ENG___<ID>`. Two
such teams share a single secrets namespace, and each reads the other's
Connections and Variables.

Whether the two rows can coexist at all is decided by the database's collation
on a `String` primary key rather than by Airflow, so the same configuration
behaves differently across backends. Rejecting the collision at creation makes
the behaviour uniform and fails at the moment the name is chosen, rather than
silently merging two teams' secrets later.

Both creation paths are covered. `teams sync` checks the incoming names against
each other as well as against the stored ones, because a bundle config can
introduce both halves of a collision in the same run.

`upper()` rather than `casefold()`: it is what the backend applies, and team
names are restricted to ASCII by the name pattern.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant