Skip to content

Only resolve a team namespaced environment secret for its own team - #70736

Merged
potiuk merged 2 commits into
apache:mainfrom
potiuk:resolve-team-namespaced-env-secret-only-for-its-team
Aug 1, 2026
Merged

Only resolve a team namespaced environment secret for its own team#70736
potiuk merged 2 commits into
apache:mainfrom
potiuk:resolve-team-namespaced-env-secret-only-for-its-team

Conversation

@potiuk

@potiuk potiuk commented Jul 30, 2026

Copy link
Copy Markdown
Member

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.

_is_team_specific_accessed_as_global guarded that, and had two gaps:

  • its pattern _[^_]+___.+ could not span an underscore in the team name, and
    team names may contain underscores (^[a-zA-Z0-9_-]{3,50}$);
  • it only applied when no team was in scope (team_name is None), so with a team
    in scope it did nothing. The team scoped probe above it only returns on a hit,
    so a miss fell through to os.environ.get(PREFIX + id.upper()) — byte identical
    to another team's variable name.

Approach

The lookup order is inverted, and the guard stops trying to attribute an id to a
team:

  1. the team scoped lookup runs first. It is safe by construction — it can only
    ever build PREFIX + "_" + <caller's team> + "___" + id, the caller's own
    namespace;
  2. after it misses, an id that spells out any team namespace (_.+___.+) is
    refused, because the team agnostic lookup would land inside one;
  3. otherwise the team agnostic lookup proceeds as before.

The id is never parsed to decide which team it belongs to, because that question
has no answer.
A team name may itself contain the ___ separator, so
_a___b___c is simultaneously team a with id b___c and team a___b with id
c, and nothing in the string distinguishes them.

An earlier revision of this PR did try to answer it, by comparing the id against
the namespace prefix the caller's own team builds and treating a match as proof of
ownership. That is not sound, and it left the cross-team read open for a whole
class of team names: for a caller in team a, the id _a___b___c starts with
_A___, so the guard cleared it — but the variable it resolves,
AIRFLOW_CONN__A___B___C, is team a___b's. The prefix matched, the team scoped
lookup missed, and the team agnostic lookup returned the other team's secret. Any
team whose name extends the caller's was readable. The regression test
test_team_whose_name_extends_the_callers_is_not_readable covers exactly that
shape.

Both get_conn_value and get_variable share the helper, so this covers
Connections and Variables together.

Behaviour change

Ids of the shape _<something>___<something> are no longer resolvable through the
team agnostic environment variable name from any scope — including the team
that owns the namespace, which previously could reach its own secret through the
namespaced spelling. That spelling is exactly what made a prefix match look like
ownership. Callers reach their own team's secrets the normal way, with the bare id
plus their team scope, which is unaffected.

Practically: a deployment that stored a team agnostic connection or variable
whose id begins with _ and contains ___ loses that lookup. Legitimate global
ids and same team lookups are unaffected.

Test plan

  • airflow-core/tests/unit/always/test_secrets_environment_variables.py, 34 cases —
    every test parametrized over both lookups and over an underscore-bearing (team_a)
    and underscore-free (teama) team name
  • Negative: not resolved without a team scope; not resolved for another team; not
    resolved through the namespaced spelling by any caller including the owning team
  • test_team_whose_name_extends_the_callers_is_not_readable — caller team_a,
    target team_a___prod, id _team_a___prod___dbconn; asserts the read is refused
    and that team_a___prod still reaches its own secret with the bare id
  • Positive: resolved for its own team; team scoped wins over team agnostic; team
    agnostic still resolves for any team scope
  • The 6 cases covering the collision fail against the previous revision of this PR
  • Separately, an exhaustive property check over 6 team names (including the
    collision-prone team_a / team_a___prod and a / a___b), both lookups, every
    caller scope including none, and both bare and namespaced ids: 0 cross-team
    resolutions
    with this change, 4 with the previous revision
  • ruff check / ruff format --check clean on both files
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

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
@potiuk
potiuk requested review from ashb and dstandish as code owners July 30, 2026 09:41
@potiuk potiuk added this to the Airflow 3.3.1 milestone Jul 30, 2026
@potiuk potiuk added the backport-to-v3-3-test Backport to v3-3-test label Jul 30, 2026
@potiuk
potiuk requested review from vincbeck and removed request for ashb and dstandish July 30, 2026 09:41
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.
@potiuk
potiuk merged commit e0cac1f into apache:main Aug 1, 2026
78 checks passed
@potiuk
potiuk deleted the resolve-team-namespaced-env-secret-only-for-its-team branch August 1, 2026 00:47
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-3-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
v3-3-test PR Link

github-actions Bot pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Aug 1, 2026
… own team (apache#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>
potiuk added a commit that referenced this pull request Aug 1, 2026
… own team (#70736) (#70882)

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants