This is an automated email from the ASF dual-hosted git repository.
eladkal pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new cf76cd0c8e1 Refuse Akeyless secret ids that address another team's
namespace (#72646)
cf76cd0c8e1 is described below
commit cf76cd0c8e1592c26ee3fe8414a2e4ea5025f5ec
Author: Jarek Potiuk <[email protected]>
AuthorDate: Thu Sep 10 06:51:44 2026 +0200
Refuse Akeyless secret ids that address another team's namespace (#72646)
Secret names are built by joining <base path><sep><key>, and the key was not
validated. In multi-team mode the team-scoped lookup is tried first and,
when
it misses, the team-agnostic fallback resolves <base path><sep><key> -- the
prefix under which every other team's secrets are stored. A caller in team
alpha requesting the key 'beta/db_password' therefore reached team beta's
secret. The key is Dag-author controlled and the execution API variables
route
is declared with a ':path' converter, so a separator survives the round
trip.
Refuse such a key only where this backend actually crosses a namespace: when
multi-team mode is on, team-scoped paths are in use, and a team name was
supplied. The separator here is the ordinary path separator and nested keys
are a documented layout, so the refusal is kept as narrow as the defect:
* use_team_secrets_path=False builds no team path, so nothing is refused and
a flat nested layout keeps working under multi-team mode.
* get_config takes no team name and Airflow does not perform team-scoped
config lookups through a secrets backend, so that path has no boundary to
cross and is not guarded; subfolder config layouts keep resolving.
* A caller with no team name resolves in the shared namespace directly
rather
than falling back into it. Whether global scope should be able to name a
team's namespace is a separate question and is not decided here.
The key is never parsed to determine which team it names, because it cannot
be -- nothing distinguishes a nested key in the shared namespace from one
naming another team.
Also read core.multi_team with getboolean. It was read with conf.get, which
returns the string 'False' -- truthy -- so the multi-team branches were
selected even with multi-team disabled.
The escape tests wire the backend so the cross-team path would return a
value
and assert it does not come back. Against unpatched sources they fail with
"assert 'beta-secret' is None".
---
.../airflow/providers/akeyless/secrets/akeyless.py | 61 ++++++++-
.../tests/unit/akeyless/secrets/test_akeyless.py | 136 +++++++++++++++++++++
2 files changed, 196 insertions(+), 1 deletion(-)
diff --git
a/providers/akeyless/src/airflow/providers/akeyless/secrets/akeyless.py
b/providers/akeyless/src/airflow/providers/akeyless/secrets/akeyless.py
index eedc6794810..e40b9318203 100644
--- a/providers/akeyless/src/airflow/providers/akeyless/secrets/akeyless.py
+++ b/providers/akeyless/src/airflow/providers/akeyless/secrets/akeyless.py
@@ -181,6 +181,55 @@ class AkeylessBackend(BaseSecretsBackend, LoggingMixin):
return cid.generateAzure(self._extra.get("azure_object_id"))
raise ValueError(f"No cloud-id generator for {self._access_type!r}")
+ def _multi_team_enabled(self) -> bool:
+ """Whether the deployment runs in multi-team mode."""
+ return conf.getboolean("core", "multi_team", fallback=False)
+
+ def _escapes_its_namespace(self, key: str, team_name: str | None) -> bool:
+ """
+ Whether looking ``key`` up for ``team_name`` could resolve another
team's secret.
+
+ Only the team-scoped lookup crosses a namespace boundary. It is tried
under
+ ``<base path><sep><team><sep><key>`` and, when that misses, falls back
to
+ ``<base path><sep><key>`` -- the prefix every *other* team's secrets
sit under. A
+ caller in team ``alpha`` asking for ``beta<sep>db_password`` therefore
reaches team
+ ``beta``'s secret through the fallback. The key is Dag-author
controlled and the
+ execution API variables route is declared with a ``:path`` converter,
so a separator
+ survives the round trip.
+
+ The refusal is deliberately narrow, because in this backend the
separator is the
+ ordinary path separator and nested keys are a legitimate, documented
layout. It
+ applies only when this backend actually builds a team-scoped path and
can fall back
+ past it:
+
+ * ``use_team_secrets_path=False`` disables team-scoped lookup
entirely, so no team
+ path is constructed and no boundary is crossed -- nested keys keep
working.
+ * A caller with no ``team_name`` resolves in the shared namespace
directly rather
+ than falling back into it. Whether a global-scope caller should be
able to name a
+ team's namespace is a separate question about global scope, not this
fallback, and
+ is left alone here.
+ * Outside multi-team mode there are no team namespaces at all.
+
+ The key is never parsed to work out *which* team it names, because it
cannot be:
+ nothing distinguishes a nested key in the shared namespace from one
naming a team.
+ """
+ return (
+ self._multi_team_enabled()
+ and self.use_team_secrets_path
+ and team_name is not None
+ and self.sep in key
+ )
+
+ def _log_refusal(self, kind: str, key: str) -> None:
+ self.log.warning(
+ "%s id %r contains %r, which separates path segments in an
Akeyless secret name. "
+ "Looked up for a team, such an id can resolve another team's
namespace through "
+ "the team-agnostic fallback, so it is not looked up. Returning
None.",
+ kind.capitalize(),
+ key,
+ self.sep,
+ )
+
def _get_secret(self, base_path: str | None, key: str) -> str | None:
if base_path is None:
return None
@@ -199,7 +248,7 @@ class AkeylessBackend(BaseSecretsBackend, LoggingMixin):
"""Look up a secret with team-scoped path, falling back to global."""
if base_path is None:
return None
- multi_team = conf.get("core", "multi_team", fallback=False)
+ multi_team = self._multi_team_enabled()
if multi_team and self.use_team_secrets_path and team_name is not None:
team_path = f"{base_path}{self.sep}{team_name}"
response = self._get_secret(team_path, key)
@@ -217,6 +266,9 @@ class AkeylessBackend(BaseSecretsBackend, LoggingMixin):
"""Build a ``Connection`` from an Akeyless secret (URI or JSON
dict)."""
from airflow.models.connection import Connection
+ if self._escapes_its_namespace(conn_id, team_name):
+ self._log_refusal("connection", conn_id)
+ return None
raw = self._get_team_or_global_secret(self.connections_path,
team_name, conn_id)
if raw is None:
return None
@@ -231,6 +283,9 @@ class AkeylessBackend(BaseSecretsBackend, LoggingMixin):
def get_variable(self, key: str, team_name: str | None = None) -> str |
None:
"""Retrieve an Airflow Variable from Akeyless."""
+ if self._escapes_its_namespace(key, team_name):
+ self._log_refusal("variable", key)
+ return None
raw = self._get_team_or_global_secret(self.variables_path, team_name,
key)
if raw is None:
return None
@@ -244,6 +299,10 @@ class AkeylessBackend(BaseSecretsBackend, LoggingMixin):
def get_config(self, key: str) -> str | None:
"""Retrieve an Airflow Configuration option from Akeyless."""
+ # No guard here. Config lookups carry no team_name and Airflow does
not perform
+ # team-scoped config lookups through a secrets backend, so this path
never builds a
+ # team-scoped name and has no boundary to cross. Refusing nested ids
here would only
+ # break subfolder config layouts.
raw = self._get_secret(self.config_path, key)
if raw is None:
return None
diff --git a/providers/akeyless/tests/unit/akeyless/secrets/test_akeyless.py
b/providers/akeyless/tests/unit/akeyless/secrets/test_akeyless.py
index eb97bfecb38..7336f78f091 100644
--- a/providers/akeyless/tests/unit/akeyless/secrets/test_akeyless.py
+++ b/providers/akeyless/tests/unit/akeyless/secrets/test_akeyless.py
@@ -392,3 +392,139 @@ class TestAkeylessBackend:
with patch.dict(sys.modules, {"akeyless_cloud_id": None}):
with pytest.raises(ImportError, match="akeyless_cloud_id"):
backend._get_cloud_id()
+
+ # ------------------------------------------------------------------
+ # Cross-team namespace escape
+ # ------------------------------------------------------------------
+
+ @patch(f"{BACKEND_MODULE}.akeyless")
+ def test_get_variable_cannot_reach_another_teams_namespace(self, mock_sdk):
+ """A key containing the separator must not resolve another team's
secret.
+
+ The team-scoped lookup misses and the team-agnostic fallback resolves
+ ``{base}/{key}`` -- which is the prefix every other team's secrets
live under.
+ The backend is wired here so that the cross-team path *would* return a
value,
+ so the assertion is that team beta's secret does not come back, not
merely
+ that some guard ran.
+ """
+ api = mock_sdk.V2Api.return_value
+ api.auth.return_value = MagicMock(token="t")
+ mock_sdk.ApiException = Exception
+ api.get_secret_value.return_value =
{"/airflow/variables/beta/db_password": "beta-secret"}
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ backend = _backend()
+ val = backend.get_variable("beta/db_password", team_name="alpha")
+
+ assert val is None
+ api.get_secret_value.assert_not_called()
+
+ @patch(f"{BACKEND_MODULE}.akeyless")
+ def test_get_connection_cannot_reach_another_teams_namespace(self,
mock_sdk):
+ """The same escape is refused for connections."""
+ api = mock_sdk.V2Api.return_value
+ api.auth.return_value = MagicMock(token="t")
+ mock_sdk.ApiException = Exception
+ api.get_secret_value.return_value =
{"/airflow/connections/beta/prod_db": "postgres://u:p@h/d"}
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ backend = _backend()
+ conn = backend.get_connection("beta/prod_db", team_name="alpha")
+
+ assert conn is None
+ api.get_secret_value.assert_not_called()
+
+ @patch(f"{BACKEND_MODULE}.akeyless")
+ def test_nested_config_ids_still_resolve_in_multi_team_mode(self,
mock_sdk):
+ """Config lookups are global and must keep working with subfolder
layouts.
+
+ ``get_config`` takes no ``team_name`` and Airflow does not do
team-scoped config
+ lookups through a secrets backend, so there is no boundary for a
config id to
+ cross. Guarding it would silently break subfolder config layouts on
upgrade.
+ """
+ api = mock_sdk.V2Api.return_value
+ api.auth.return_value = MagicMock(token="t")
+ api.get_secret_value.return_value =
{"/airflow/config/db/sql_alchemy_conn": "postgres://x"}
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ backend = _backend()
+ val = backend.get_config("db/sql_alchemy_conn")
+
+ assert val == "postgres://x"
+
+ @patch(f"{BACKEND_MODULE}.akeyless")
+ def test_nested_keys_still_resolve_when_team_paths_are_disabled(self,
mock_sdk):
+ """``use_team_secrets_path=False`` builds no team path, so nothing is
refused.
+
+ A deployment can run multi-team for other features while keeping
Akeyless as a
+ single flat namespace. Banning separators there would be a pure
regression.
+ """
+ api = mock_sdk.V2Api.return_value
+ api.auth.return_value = MagicMock(token="t")
+ api.get_secret_value.return_value =
{"/airflow/variables/nested/my_var": "nested-val"}
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ backend = _backend(use_team_secrets_path=False)
+ val = backend.get_variable("nested/my_var", team_name="alpha")
+
+ assert val == "nested-val"
+
+ @patch(f"{BACKEND_MODULE}.akeyless")
+ def test_nested_keys_still_resolve_for_a_caller_with_no_team(self,
mock_sdk):
+ """With no team_name the lookup resolves in the shared namespace
directly."""
+ api = mock_sdk.V2Api.return_value
+ api.auth.return_value = MagicMock(token="t")
+ api.get_secret_value.return_value =
{"/airflow/variables/nested/my_var": "nested-val"}
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ backend = _backend()
+ val = backend.get_variable("nested/my_var")
+
+ assert val == "nested-val"
+
+ @patch(f"{BACKEND_MODULE}.akeyless")
+ def test_a_team_scoped_key_without_the_separator_still_resolves(self,
mock_sdk):
+ """The guard must not break ordinary team-scoped lookups."""
+ api = mock_sdk.V2Api.return_value
+ api.auth.return_value = MagicMock(token="t")
+ api.get_secret_value.return_value =
{"/airflow/variables/alpha/my_var": "team-val"}
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ backend = _backend()
+ val = backend.get_variable("my_var", team_name="alpha")
+
+ assert val == "team-val"
+
+ @patch(f"{BACKEND_MODULE}.akeyless")
+ def test_nested_keys_still_work_outside_multi_team_mode(self, mock_sdk):
+ """Without team namespaces a separator in a key is an ordinary nested
path."""
+ api = mock_sdk.V2Api.return_value
+ api.auth.return_value = MagicMock(token="t")
+ api.get_secret_value.return_value =
{"/airflow/variables/nested/my_var": "nested-val"}
+
+ with conf_vars({("core", "multi_team"): "False"}):
+ backend = _backend()
+ val = backend.get_variable("nested/my_var")
+
+ assert val == "nested-val"
+
+ @patch(f"{BACKEND_MODULE}.akeyless")
+ def test_multi_team_disabled_is_read_as_a_boolean(self, mock_sdk):
+ """``multi_team = False`` must not select the multi-team code paths.
+
+ The option was previously read with ``conf.get``, which yields the
string
+ ``"False"`` -- truthy -- so the global-path branch was taken even with
+ multi-team off.
+ """
+ api = mock_sdk.V2Api.return_value
+ api.auth.return_value = MagicMock(token="t")
+ api.get_secret_value.return_value = {"/airflow/variables/my_var":
"plain-val"}
+
+ with conf_vars({("core", "multi_team"): "False"}):
+ backend = _backend(global_secrets_path="global")
+ val = backend.get_variable("my_var")
+
+ assert val == "plain-val"
+ # The global-secrets path must not have been consulted at all.
+ requested = [c.kwargs["names"][0] for c in
mock_sdk.GetSecretValue.call_args_list]
+ assert requested == ["/airflow/variables/my_var"]
