This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch v3-3-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v3-3-test by this push:
new a033b7a6c61 [v3-3-test] Refuse a separator-bearing secret id before
the team scoped lookup (#70902) (#71041)
a033b7a6c61 is described below
commit a033b7a6c613c79f495b9acce5221265fc7c9649
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue Aug 4 02:46:58 2026 +0200
[v3-3-test] Refuse a separator-bearing secret id before the team scoped
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 ed87a1a024a55aaa494e6a718cb876f0cbe47b44)
Co-authored-by: Jarek Potiuk <[email protected]>
---
.../src/airflow/secrets/environment_variables.py | 46 ++++++----------------
.../always/test_secrets_environment_variables.py | 44 +++++++++++++++++++++
2 files changed, 57 insertions(+), 33 deletions(-)
diff --git a/airflow-core/src/airflow/secrets/environment_variables.py
b/airflow-core/src/airflow/secrets/environment_variables.py
index 1a50d58138a..2432c084f5d 100644
--- a/airflow-core/src/airflow/secrets/environment_variables.py
+++ b/airflow-core/src/airflow/secrets/environment_variables.py
@@ -20,27 +20,32 @@
from __future__ import annotations
import os
-import re
from airflow.secrets import BaseSecretsBackend
CONN_ENV_PREFIX = "AIRFLOW_CONN_"
VAR_ENV_PREFIX = "AIRFLOW_VAR_"
+# Separates the team name from the secret id in a team namespaced environment
variable
+# name: AIRFLOW_CONN__<TEAM>___<ID>.
+TEAM_SEP = "___"
+
class EnvironmentVariablesBackend(BaseSecretsBackend):
"""Retrieves Connection object and Variable from environment variable."""
def get_conn_value(self, conn_id: str, team_name: str | None = None) ->
str | None:
+ if TEAM_SEP in conn_id:
+ # An id containing the separator could collide with another team's
namespace
+ # even on the scoped lookup below, so it must be refused before
either runs.
+ return None
+
if team_name and (
team_var :=
os.environ.get(f"{CONN_ENV_PREFIX}_{team_name.upper()}___" + conn_id.upper())
):
# Format to set a team specific connection:
AIRFLOW_CONN__<TEAM_ID>___<CONN_ID>
return team_var
- if self._names_a_team_namespace(conn_id):
- return None
-
return os.environ.get(CONN_ENV_PREFIX + conn_id.upper())
def get_variable(self, key: str, team_name: str | None = None) -> str |
None:
@@ -51,39 +56,14 @@ class EnvironmentVariablesBackend(BaseSecretsBackend):
:param team_name: Team name associated to the task trying to access
the variable (if any)
:return: Variable Value
"""
+ if TEAM_SEP in key:
+ # Same collision risk as get_conn_value, see its code comment.
+ return None
+
if team_name and (
team_var :=
os.environ.get(f"{VAR_ENV_PREFIX}_{team_name.upper()}___" + key.upper())
):
# Format to set a team specific variable:
AIRFLOW_VAR__<TEAM_ID>___<VAR_KEY>
return team_var
- if self._names_a_team_namespace(key):
- return None
-
return os.environ.get(VAR_ENV_PREFIX + key.upper())
-
- @staticmethod
- def _names_a_team_namespace(secret_id: str) -> bool:
- """
- Whether ``secret_id`` spells out a team namespaced environment
variable name.
-
- A team specific secret lives in the ``_<TEAM_NAME>___<SECRET_ID>``
namespace of the
- environment. An id of that shape therefore makes the team agnostic
lookup -- which
- prepends only ``AIRFLOW_CONN_`` / ``AIRFLOW_VAR_`` -- land inside some
team's namespace,
- so that lookup is refused for such an id.
-
- **The id is never attributed to a particular team**, because it cannot
be: a team name
- may itself contain the ``___`` separator, so ``_a___b___c`` is both
team ``a`` with id
- ``b___c`` and team ``a___b`` with id ``c``, and nothing in the string
distinguishes them.
- Only the caller's own namespace is ever constructed, never parsed.
-
- This is why the check is not "does the id belong to some team other
than the caller's".
- Comparing the id against the prefix the caller's team builds looks
equivalent and is not:
- for a caller in team ``a`` the id ``_a___b___c`` starts with
``_A___``, yet the variable
- it resolves, ``AIRFLOW_CONN__A___B___C``, is team ``a___b``'s.
Treating a prefix match as
- ownership hands one team the secrets of every team whose name extends
it. Callers reach
- their own team's secrets with the bare id plus their team scope --
handled above, and safe
- because that path can only ever build the caller's own namespace -- so
nothing legitimate
- needs a namespaced id here.
- """
- return re.fullmatch(r"_.+___.+", secret_id) is not None
diff --git
a/airflow-core/tests/unit/always/test_secrets_environment_variables.py
b/airflow-core/tests/unit/always/test_secrets_environment_variables.py
index 18fb5d2444e..9f8e8379efc 100644
--- a/airflow-core/tests/unit/always/test_secrets_environment_variables.py
+++ b/airflow-core/tests/unit/always/test_secrets_environment_variables.py
@@ -21,6 +21,7 @@ import pytest
from airflow.secrets.environment_variables import (
CONN_ENV_PREFIX,
+ TEAM_SEP,
VAR_ENV_PREFIX,
EnvironmentVariablesBackend,
)
@@ -136,6 +137,49 @@ class TestEnvironmentVariablesBackendTeamScope:
assert lookup(env_prefix, method, SECRET_ID, team_name) == GLOBAL_VALUE
+ @pytest.mark.parametrize(("env_prefix", "method"), LOOKUPS)
+ def test_a_bare_id_containing_the_separator_is_not_resolved(self,
monkeypatch, env_prefix, method):
+ """The scoped lookup can build another team's variable name from a
*bare* id.
+
+ Caller in ``team_a`` supplies ``prod___dbconn``. The team scoped
lookup builds
+ ``<PREFIX>_TEAM_A___PROD___DBCONN`` -- byte-identical to what team
``team_a___prod``
+ builds for its own id ``dbconn``. That lookup **hits**, so refusing
only the team
+ agnostic fall-through never sees this: the fall-through is never
reached.
+ """
+ target_team, target_id = "team_a___prod", "dbconn"
+ monkeypatch.setenv(team_env_var(env_prefix, target_team, target_id),
TEAM_VALUE)
+
+ assert lookup(env_prefix, method, f"prod{TEAM_SEP}{target_id}",
"team_a") is None
+
+ @pytest.mark.parametrize(("env_prefix", "method"), LOOKUPS)
+ @pytest.mark.parametrize("team_name", [None, *TEAM_NAMES])
+ def test_an_id_containing_the_separator_is_refused_in_every_scope(
+ self, monkeypatch, env_prefix, method, team_name
+ ):
+ """One rule, applied the same way with or without a team scope.
+
+ An id carrying the separator cannot be resolved unambiguously in
either direction, so
+ there is no scope in which it is safe to look up.
+ """
+ monkeypatch.setenv(env_prefix + f"A{TEAM_SEP}B".upper(), GLOBAL_VALUE)
+
+ assert lookup(env_prefix, method, f"a{TEAM_SEP}b", team_name) is None
+
+ @pytest.mark.parametrize(("env_prefix", "method"), LOOKUPS)
+ @pytest.mark.parametrize("team_name", TEAM_NAMES)
+ def test_the_owning_team_cannot_reach_a_separator_bearing_id_either(
+ self, monkeypatch, env_prefix, method, team_name
+ ):
+ """Documents the cost of the rule, so a future change does not
reintroduce the hole.
+
+ A team's *own* secret whose id contains the separator is unreachable
too. That is
+ deliberate: the string it builds is the same one another team's name
could build, and
+ nothing in it says which reading is intended.
+ """
+ monkeypatch.setenv(team_env_var(env_prefix, team_name,
f"prod{TEAM_SEP}dbconn"), TEAM_VALUE)
+
+ assert lookup(env_prefix, method, f"prod{TEAM_SEP}dbconn", team_name)
is None
+
@pytest.mark.parametrize(("env_prefix", "method"), LOOKUPS)
@pytest.mark.parametrize("team_name", [None, *TEAM_NAMES])
def test_unset_secret_is_not_resolved(self, monkeypatch, env_prefix,
method, team_name):
