potiuk opened a new pull request, #70902:
URL: https://github.com/apache/airflow/pull/70902
The team scoped lookup builds `PREFIX + _<TEAM>___ + <ID>`, so a secret id
that
itself contains `___` makes that string ambiguous:
```
caller in team "team_a", bare id "prod___dbconn"
AIRFLOW_CONN_ + _TEAM_A + ___ + PROD___DBCONN ->
AIRFLOW_CONN__TEAM_A___PROD___DBCONN
team "team_a___prod", its own id "dbconn"
AIRFLOW_CONN_ + _TEAM_A___PROD + ___ + DBCONN ->
AIRFLOW_CONN__TEAM_A___PROD___DBCONN
```
Byte-identical — and the **scoped** lookup hits, so the existing guard,
which ran
only ahead of the team agnostic fall-through, was never reached for this
shape.
### Approach
Two changes, both small:
1. **Move the check ahead of both lookups** rather than only the
fall-through.
2. **Widen it** from "spells out a team namespace" (`_<TEAM>___<ID>`) to
simply
"contains the separator".
The narrower form had to reason about *which* team an id might name. That is
unanswerable while a team name may itself contain the separator, and it made
the
guard depend on every stored team name being valid — which `teams sync` has
never
enforced. Asking only "does this contain `___`" removes both problems, and
drops
`_names_a_team_namespace` entirely.
This is also what the provider secrets backends already do (`TEAM_SEP in
secret_id`, applied to every getter). Core was the outlier.
### Behaviour change
A Connection or Variable whose **id** contains `___` is no longer resolvable
from
the environment backend — in either scope, including for its owning team.
That is deliberate. The name such an id builds is one another team's name
could
build, and nothing in the string says which reading was meant. The owning
team
reaches its own secret the normal way, with the bare id plus its team scope,
which
is unaffected. Ids without the separator are unaffected in every scope.
### Test plan
- [x] `test_a_bare_id_containing_the_separator_is_not_resolved` — the
collision
above, both lookups
- [x] `test_an_id_containing_the_separator_is_refused_in_every_scope` — one
rule,
with and without a team scope
- [x] `test_the_owning_team_cannot_reach_a_separator_bearing_id_either` —
pins the
cost, so a later relaxation cannot quietly reopen the hole
- [x] Verified against the code with only the two guards removed (constant
kept, so
the failure is behavioural not an import error): **26 failed**; with
them,
**46 passed**
- [x] All 34 pre-existing tests in the file pass unchanged
- [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]
