[v3-3-test] Only resolve a team namespaced environment secret for its own team (#70736) - #70882
Merged
Conversation
… own team (#70736) * Only resolve a team namespaced environment secret for its own team A team specific Connection or Variable lives in the `_<TEAM_NAME>___<SECRET_ID>` namespace of the environment. An id that already spells such a namespace out therefore reaches that variable through the team agnostic lookup, which is only correct when the namespace is the one the lookup is made for. The check guarding that had two gaps. Its pattern, `_[^_]+___.+`, could not span an underscore in the team name, and team names may contain underscores. And it only applied when no team was in scope, so with a team in scope it did nothing: the team scoped probe only returns on a hit, and a miss fell through to the team agnostic name, which is byte identical to the other team's variable. Recognise a namespaced id without assuming the team name has no underscores, deny it outright when no team is in scope, and otherwise allow it only when it begins with the namespace prefix that the team in scope itself builds. The stored id is deliberately not parsed: a team name may contain underscores, so `_a___b___c` is both team `a` with id `b___c` and team `a___b` with id `c`, and no pattern separates them. Comparing against the prefix the caller builds needs no such reading. Both lookups share the helper, so this covers Connections and Variables. 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 * Refuse a team namespaced id for the team agnostic lookup outright The previous guard compared the supplied id against the namespace prefix the caller's own team builds, and treated a match as proof the id was the caller's own. It is not. A team name may itself contain the `___` separator, so one team's namespace can start with another's: for a caller in team `a`, the id `_a___b___c` starts with `_A___`, but the variable it resolves, `AIRFLOW_CONN__A___B___C`, belongs to team `a___b`. The prefix cleared the guard, the team scoped lookup missed, and the team agnostic lookup returned the other team's secret -- the same cross-team read the guard exists to stop, for every team whose name extends the caller's. Drop the attribution attempt. The team scoped lookup runs first and is safe by construction, since it can only ever build the caller's own namespace. After it misses, an id that spells out any team namespace is refused, because the team agnostic lookup would land inside one. The id is never parsed to decide which team it belongs to -- that question has no answer. A caller reaching its own team's secret through the namespaced spelling rather than the bare id plus its team scope is no longer resolved. That spelling is what made a prefix match look like ownership. (cherry picked from commit e0cac1f) Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Contributor
Author
|
Hi maintainer, this PR was merged without a milestone set.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A team specific Connection or Variable lives in the
_<TEAM_NAME>___<SECRET_ID>namespace of the environment. An id that already spells such a namespace out
therefore reaches that variable through the team agnostic lookup, which is only
correct when the namespace is the one the lookup is made for.
The check guarding that had two gaps. Its pattern,
_[^_]+___.+, could not spanan underscore in the team name, and team names may contain underscores. And it
only applied when no team was in scope, so with a team in scope it did nothing:
the team scoped probe only returns on a hit, and a miss fell through to the team
agnostic name, which is byte identical to the other team's variable.
Recognise a namespaced id without assuming the team name has no underscores,
deny it outright when no team is in scope, and otherwise allow it only when it
begins with the namespace prefix that the team in scope itself builds. The
stored id is deliberately not parsed: a team name may contain underscores, so
_a___b___cis both teamawith idb___cand teama___bwith idc, andno pattern separates them. Comparing against the prefix the caller builds needs
no such reading.
Both lookups share the helper, so this covers Connections and Variables.
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
The previous guard compared the supplied id against the namespace prefix the
caller's own team builds, and treated a match as proof the id was the caller's
own. It is not. A team name may itself contain the
___separator, so oneteam's namespace can start with another's: for a caller in team
a, the id_a___b___cstarts with_A___, but the variable it resolves,AIRFLOW_CONN__A___B___C, belongs to teama___b. The prefix cleared theguard, the team scoped lookup missed, and the team agnostic lookup returned the
other team's secret -- the same cross-team read the guard exists to stop, for
every team whose name extends the caller's.
Drop the attribution attempt. The team scoped lookup runs first and is safe by
construction, since it can only ever build the caller's own namespace. After it
misses, an id that spells out any team namespace is refused, because the team
agnostic lookup would land inside one. The id is never parsed to decide which
team it belongs to -- that question has no answer.
A caller reaching its own team's secret through the namespaced spelling rather
than the bare id plus its team scope is no longer resolved. That spelling is
what made a prefix match look like ownership.
(cherry picked from commit e0cac1f)
Co-authored-by: Jarek Potiuk jarek@potiuk.com