[v3-3-test] Refuse a separator-bearing secret id before the team scoped lookup (#70902) - #71041
Merged
Conversation
…ed lookup (#70902) * Refuse a separator-bearing secret id before the team scoped lookup The team scoped lookup builds PREFIX + _<TEAM>___ + <ID>, so an id that itself contains ___ makes that string ambiguous. A caller in team_a asking for the bare id prod___dbconn builds AIRFLOW_CONN__TEAM_A___PROD___DBCONN, byte-identical to what team team_a___prod builds for its own id dbconn -- and that lookup hits, so the guard that ran only ahead of the team agnostic fall-through was never reached. Move the check ahead of both lookups and widen it from "spells out a team namespace" to "contains the separator". The narrower form had to reason about which team an id might name, which is unanswerable while a team name may itself contain the separator; the broader form does not, and it no longer depends on stored team names being valid. Costs an id that itself contains ___, which is now unreachable in either scope including for its owning team. That is deliberate and tested: the string such an id builds is one another team's name could build, and nothing in it says which reading was meant. This matches what the provider secrets backends already do. * Condense the collision rationale to one site Both lookups carried the same five-line explanation verbatim. (cherry picked from commit ed87a1a) Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Contributor
Author
|
Hi maintainer, this PR was merged without a milestone set.
|
vatsrahul1001
pushed a commit
that referenced
this pull request
Aug 5, 2026
…ed lookup (#70902) (#71041) * Refuse a separator-bearing secret id before the team scoped lookup The team scoped lookup builds PREFIX + _<TEAM>___ + <ID>, so an id that itself contains ___ makes that string ambiguous. A caller in team_a asking for the bare id prod___dbconn builds AIRFLOW_CONN__TEAM_A___PROD___DBCONN, byte-identical to what team team_a___prod builds for its own id dbconn -- and that lookup hits, so the guard that ran only ahead of the team agnostic fall-through was never reached. Move the check ahead of both lookups and widen it from "spells out a team namespace" to "contains the separator". The narrower form had to reason about which team an id might name, which is unanswerable while a team name may itself contain the separator; the broader form does not, and it no longer depends on stored team names being valid. Costs an id that itself contains ___, which is now unreachable in either scope including for its owning team. That is deliberate and tested: the string such an id builds is one another team's name could build, and nothing in it says which reading was meant. This matches what the provider secrets backends already do. * Condense the collision rationale to one site Both lookups carried the same five-line explanation verbatim. (cherry picked from commit ed87a1a) Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
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
2 participants
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.
The team scoped lookup builds PREFIX + __ + , so an id that itself
contains ___ makes that string ambiguous. A caller in team_a asking for the bare
id prod___dbconn builds AIRFLOW_CONN__TEAM_A___PROD___DBCONN, byte-identical to
what team team_a___prod builds for its own id dbconn -- and that lookup hits, so
the guard that ran only ahead of the team agnostic fall-through was never
reached.
Move the check ahead of both lookups and widen it from "spells out a team
namespace" to "contains the separator". The narrower form had to reason about
which team an id might name, which is unanswerable while a team name may itself
contain the separator; the broader form does not, and it no longer depends on
stored team names being valid.
Costs an id that itself contains ___, which is now unreachable in either scope
including for its owning team. That is deliberate and tested: the string such an
id builds is one another team's name could build, and nothing in it says which
reading was meant.
This matches what the provider secrets backends already do.
Both lookups carried the same five-line explanation verbatim.
(cherry picked from commit ed87a1a)
Co-authored-by: Jarek Potiuk jarek@potiuk.com