This is an automated email from the ASF dual-hosted git repository.
potiuk 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 a1109f86030 Mask provider-declared sensitive connection extras on
every connection type (#73794)
a1109f86030 is described below
commit a1109f86030199a4b71aa34c9ff9edaa86f2e39a
Author: Jarek Potiuk <[email protected]>
AuthorDate: Mon Oct 19 16:32:29 2026 +0200
Mask provider-declared sensitive connection extras on every connection type
(#73794)
* Mask provider-declared sensitive connection extras by connection type
The connections API masks ``extra`` values by field name only, so a
field a provider declares as a secret (``format: password`` in its
``conn-fields``) is returned in cleartext when its name is too generic
to be on the global list -- ``config`` is a secret for apprise and
harmless for other connection types. After the name-based masking, also
mask the fields the connection type's own provider declares sensitive,
read from provider metadata without importing hooks.
Generated-by: Claude Opus 5
* Add newsfragment
Generated-by: Claude Opus 5
* Restore provider-declared sensitive extras when saving a masked connection
Generated-by: Claude Opus 5
* Restore provider-sensitive extras only when the connection type is
unchanged
Generated-by: Claude Opus 5
* Keep provider-declared sensitive extras masked after a connection type
change
Generated-by: Claude Opus 5
* Describe masking of provider-declared sensitive extras across connection
types
Generated-by: Claude Opus 5
* Look up provider-declared sensitive extra keys in one set
Checking each key against every connection type's declared fields did the
same work on every key of every response; building the plain and prefixed names
once keeps the lookup a single membership test.
Generated-by: Claude Opus 5
---
airflow-core/newsfragments/73794.improvement.rst | 1 +
.../api_fastapi/core_api/datamodels/connections.py | 57 +++++++++
.../core_api/services/public/connections.py | 12 +-
.../core_api/datamodels/test_connections.py | 60 ++++++++++
.../core_api/routes/public/test_connections.py | 133 +++++++++++++++++++++
5 files changed, 261 insertions(+), 2 deletions(-)
diff --git a/airflow-core/newsfragments/73794.improvement.rst
b/airflow-core/newsfragments/73794.improvement.rst
new file mode 100644
index 00000000000..f408e652173
--- /dev/null
+++ b/airflow-core/newsfragments/73794.improvement.rst
@@ -0,0 +1 @@
+Mask connection ``extra`` fields that any installed provider declares as
secrets (``format: password`` in its connection form fields, for example
Apprise ``config``) in API responses, whatever the connection's type, in
addition to fields whose names match the global sensitive-name list. When a
client sends such a masked value back without changing the connection type, the
stored value is kept unchanged.
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py
b/airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py
index 34cfe473348..f9fa3465d07 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py
@@ -20,6 +20,7 @@ from __future__ import annotations
import json
from collections.abc import Iterable, Mapping
from datetime import datetime
+from functools import cache
from typing import Annotated, Any
from pydantic import Field, field_validator, model_validator
@@ -29,6 +30,48 @@ from airflow._shared.secrets_masker import redact,
should_hide_value_for_key
from airflow.api_fastapi.core_api.base import BaseModel, StrictBaseModel,
make_partial_model
from airflow.configuration import conf
+_MASK = "***"
+
+
+@cache
+def _sensitive_extra_keys() -> frozenset[str]:
+ """
+ Return the ``extra`` keys any provider declares sensitive, as plain and
``extra__<type>__`` names.
+
+ Providers mark a connection field as a secret with ``format: password`` in
their
+ ``conn-fields``. The name-based masking in ``redact`` cannot use that
knowledge, because it
+ only knows a fixed list of secret names, and ``config`` is not one of
them. Read from provider
+ metadata, without importing any hook.
+
+ Not limited to the connection's current type: after a ``conn_type`` change
the stored
+ ``extra`` still holds the previous type's secrets, and the new type does
not declare them.
+ """
+ from airflow.providers_manager import ProvidersManager
+
+ keys: set[str] = set()
+ for prefixed_name, widget in
ProvidersManager()._connection_form_widgets_from_metadata.items():
+ if widget.is_sensitive:
+ keys.update((widget.field_name, prefixed_name))
+ return frozenset(keys)
+
+
+def restore_provider_sensitive_extra(extra: Any, stored_extra: Any) -> Any:
+ """
+ Put back the stored value of provider-declared sensitive ``extra`` fields
submitted as the mask.
+
+ ``merge`` only restores keys on its fixed list of secret names, so the
keys masked by
+ ``ConnectionResponse.redact_provider_sensitive_extra`` would otherwise be
saved as the mask
+ when a client sends a response's ``extra`` back.
+ """
+ if not isinstance(extra, dict) or not isinstance(stored_extra, dict):
+ return extra
+ return {
+ key: stored_extra[key]
+ if value == _MASK and key in stored_extra and key in
_sensitive_extra_keys()
+ else value
+ for key, value in extra.items()
+ }
+
# Response Models
class ConnectionResponse(BaseModel):
@@ -52,6 +95,20 @@ class ConnectionResponse(BaseModel):
return None
return str(redact(v, field_info.field_name))
+ @model_validator(mode="after")
+ def redact_provider_sensitive_extra(self) -> ConnectionResponse:
+ """Mask the ``extra`` fields any provider declares sensitive."""
+ if not self.extra or not _sensitive_extra_keys():
+ return self
+ extra = json.loads(self.extra)
+ if not isinstance(extra, dict):
+ return self
+ for key, value in extra.items():
+ if key in _sensitive_extra_keys() and value not in (None, ""):
+ extra[key] = _MASK
+ self.extra = json.dumps(extra)
+ return self
+
@field_validator("extra", mode="before")
@classmethod
def redact_extra(cls, v: str | None) -> str | None:
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/services/public/connections.py
b/airflow-core/src/airflow/api_fastapi/core_api/services/public/connections.py
index 74f5d05df77..319d97204c3 100644
---
a/airflow-core/src/airflow/api_fastapi/core_api/services/public/connections.py
+++
b/airflow-core/src/airflow/api_fastapi/core_api/services/public/connections.py
@@ -32,7 +32,10 @@ from airflow.api_fastapi.core_api.datamodels.common import (
BulkDeleteAction,
BulkUpdateAction,
)
-from airflow.api_fastapi.core_api.datamodels.connections import ConnectionBody
+from airflow.api_fastapi.core_api.datamodels.connections import (
+ ConnectionBody,
+ restore_provider_sensitive_extra,
+)
from airflow.api_fastapi.core_api.services.public.common import BulkService
from airflow.models.connection import Connection
@@ -41,6 +44,7 @@ def update_orm_from_pydantic(
orm_conn: Connection, pydantic_conn: ConnectionBody, update_mask:
list[str] | None = None
) -> None:
"""Update ORM object from Pydantic object."""
+ stored_conn_type = orm_conn.conn_type
# Not all fields match and some need setters, therefore copy partly
manually via setters
non_update_fields = {"connection_id", "conn_id"}
setter_fields = {"password", "extra"}
@@ -71,7 +75,11 @@ def update_orm_from_pydantic(
orm_conn.set_extra(pydantic_conn.extra)
return
try:
- merged_extra = merge(json.loads(pydantic_conn.extra),
json.loads(orm_conn.extra))
+ old_extra = json.loads(orm_conn.extra)
+ merged_extra = merge(json.loads(pydantic_conn.extra), old_extra)
+ # A secret declared by one provider must not be handed to another
type's hook.
+ if orm_conn.conn_type == stored_conn_type:
+ merged_extra = restore_provider_sensitive_extra(merged_extra,
old_extra)
orm_conn.set_extra(json.dumps(merged_extra))
except json.JSONDecodeError:
# We can't merge fields in an unstructured `extra`
diff --git
a/airflow-core/tests/unit/api_fastapi/core_api/datamodels/test_connections.py
b/airflow-core/tests/unit/api_fastapi/core_api/datamodels/test_connections.py
index 072f35167fd..c563bfeef40 100644
---
a/airflow-core/tests/unit/api_fastapi/core_api/datamodels/test_connections.py
+++
b/airflow-core/tests/unit/api_fastapi/core_api/datamodels/test_connections.py
@@ -16,6 +16,8 @@
# under the License.
from __future__ import annotations
+import json
+
import pytest
from pydantic import ValidationError
@@ -56,3 +58,61 @@ def test_redact_extra_rejects_non_json(extra):
"""
with pytest.raises(ValidationError):
ConnectionResponse.model_validate(_payload(extra=extra))
+
+
+class TestRedactProviderSensitiveExtra:
+ """
+ ``extra`` fields a provider declares sensitive are masked on every
connection.
+
+ Not only on that provider's type: after a ``conn_type`` change the stored
``extra`` still holds
+ the previous type's secrets.
+ """
+
+ @pytest.fixture(autouse=True)
+ def _sensitive_fields(self, monkeypatch):
+ monkeypatch.setattr(
+
"airflow.api_fastapi.core_api.datamodels.connections._sensitive_extra_keys",
+ lambda: frozenset({"config", "extra__apprise__config"}),
+ )
+
+ def _extra(self, conn_type, extra):
+ response =
ConnectionResponse.model_validate(_payload(conn_type=conn_type,
extra=json.dumps(extra)))
+ return json.loads(response.extra)
+
+ def test_masks_declared_field_for_its_connection_type(self):
+ extra = self._extra("apprise", {"config": '{"path":
"mailto://user:pass@host"}', "tag": "alerts"})
+ assert extra == {"config": "***", "tag": "alerts"}
+
+ def test_masks_prefixed_form_of_declared_field(self):
+ extra = self._extra("apprise", {"extra__apprise__config":
"secret-config"})
+ assert extra == {"extra__apprise__config": "***"}
+
+ @pytest.mark.parametrize("key", ["config", "extra__apprise__config"])
+ def test_masks_declared_field_on_other_connection_types(self, key):
+ assert self._extra("http", {key: "secret-config"}) == {key: "***"}
+
+ def test_leaves_empty_declared_field_alone(self):
+ assert self._extra("apprise", {"config": ""}) == {"config": ""}
+
+
+def test_sensitive_extra_keys_reads_provider_declarations(monkeypatch):
+ from types import SimpleNamespace
+
+ from airflow.api_fastapi.core_api.datamodels import connections as module
+
+ widgets = {
+ "extra__apprise__config": SimpleNamespace(field_name="config",
is_sensitive=True),
+ "extra__apprise__tag": SimpleNamespace(field_name="tag",
is_sensitive=False),
+ "extra__my_type__api_secret": SimpleNamespace(field_name="api_secret",
is_sensitive=True),
+ }
+ monkeypatch.setattr(
+ "airflow.providers_manager.ProvidersManager",
+ lambda:
SimpleNamespace(_connection_form_widgets_from_metadata=widgets),
+ )
+ module._sensitive_extra_keys.cache_clear()
+ try:
+ assert module._sensitive_extra_keys() == frozenset(
+ {"config", "extra__apprise__config", "api_secret",
"extra__my_type__api_secret"}
+ )
+ finally:
+ module._sensitive_extra_keys.cache_clear()
diff --git
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
index f8735b498e1..f53932d3a00 100644
---
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
+++
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
@@ -1007,6 +1007,93 @@ class TestPatchConnection(TestConnectionEndpoint):
assert response.json() == expected_response
_check_last_log(session, dag_id=None, event="patch_connection",
logical_date=None, check_masked=True)
+ @mock.patch(
+
"airflow.api_fastapi.core_api.datamodels.connections._sensitive_extra_keys",
+ autospec=True,
+ return_value=frozenset({"config", "extra__apprise__config"}),
+ )
+ @pytest.mark.parametrize("key", ["config", "extra__apprise__config"])
+ def test_patch_keeps_masked_provider_sensitive_extra(self, _, test_client,
session, key):
+ session.add(
+ Connection(
+ conn_id=TEST_CONN_ID,
+ conn_type="apprise",
+ extra=json.dumps({key: "secret-config", "tag": "alerts"}),
+ )
+ )
+ session.commit()
+ masked_extra =
test_client.get(f"/connections/{TEST_CONN_ID}").json()["extra"]
+ assert json.loads(masked_extra) == {key: "***", "tag": "alerts"}
+
+ response = test_client.patch(
+ f"/connections/{TEST_CONN_ID}",
+ json={
+ "connection_id": TEST_CONN_ID,
+ "conn_type": "apprise",
+ "extra": json.dumps({key: "***", "tag": "updated"}),
+ },
+ )
+
+ assert response.status_code == 200
+ session.expire_all()
+ db_conn =
session.scalar(select(Connection).filter_by(conn_id=TEST_CONN_ID))
+ assert json.loads(db_conn.extra) == {key: "secret-config", "tag":
"updated"}
+
+ @mock.patch(
+
"airflow.api_fastapi.core_api.datamodels.connections._sensitive_extra_keys",
+ autospec=True,
+ return_value=frozenset({"config", "extra__apprise__config"}),
+ )
+ def
test_patch_does_not_carry_provider_sensitive_extra_to_another_type(self, _,
test_client, session):
+ session.add(
+ Connection(conn_id=TEST_CONN_ID, conn_type="apprise",
extra='{"config": "secret-config"}')
+ )
+ session.commit()
+
+ response = test_client.patch(
+ f"/connections/{TEST_CONN_ID}",
+ json={"connection_id": TEST_CONN_ID, "conn_type": "http", "extra":
'{"config": "***"}'},
+ )
+
+ assert response.status_code == 200
+ session.expire_all()
+ db_conn =
session.scalar(select(Connection).filter_by(conn_id=TEST_CONN_ID))
+ assert json.loads(db_conn.extra) == {"config": "***"}
+
+ @mock.patch(
+
"airflow.api_fastapi.core_api.datamodels.connections._sensitive_extra_keys",
+ autospec=True,
+ return_value=frozenset({"config", "extra__apprise__config"}),
+ )
+ @pytest.mark.parametrize(
+ ("extra", "params"),
+ [
+ pytest.param({}, {}, id="extra-omitted"),
+ pytest.param(
+ {"extra": '{"config": "new"}'}, {"update_mask":
["conn_type"]}, id="extra-masked-out"
+ ),
+ ],
+ )
+ def test_patch_type_change_keeps_stored_provider_sensitive_extra_masked(
+ self, _, test_client, session, extra, params
+ ):
+ session.add(
+ Connection(conn_id=TEST_CONN_ID, conn_type="apprise",
extra='{"config": "secret-config"}')
+ )
+ session.commit()
+
+ response = test_client.patch(
+ f"/connections/{TEST_CONN_ID}",
+ json={"connection_id": TEST_CONN_ID, "conn_type": "http", **extra},
+ params=params,
+ )
+
+ assert response.status_code == 200
+ assert response.json()["conn_type"] == "http"
+ assert json.loads(response.json()["extra"]) == {"config": "***"}
+ get_response = test_client.get(f"/connections/{TEST_CONN_ID}")
+ assert json.loads(get_response.json()["extra"]) == {"config": "***"}
+
def test_patch_with_update_mask_validates_extra_as_json(self, test_client):
"""When update_mask includes 'extra', the extra field validator should
still reject invalid JSON."""
self.create_connection()
@@ -1325,6 +1412,52 @@ class TestConnection(TestConnectionEndpoint):
assert json.loads(db_conn.extra) == {"path": "/", "existing_key":
"existing_value"}
assert session.scalar(select(func.count()).select_from(Connection)) ==
initial_count
+ @mock.patch(
+
"airflow.api_fastapi.core_api.datamodels.connections._sensitive_extra_keys",
+ autospec=True,
+ return_value=frozenset({"path", "extra__fs__path"}),
+ )
+ @mock.patch.dict(os.environ, {"AIRFLOW__CORE__TEST_CONNECTION": "Enabled"})
+ def
test_should_reuse_masked_provider_sensitive_extra_of_existing_connection(
+ self, _, test_client, session
+ ):
+ session.add(Connection(conn_id=TEST_CONN_ID, conn_type="fs",
extra='{"path": "/secret"}'))
+ session.commit()
+ captured_extra = {}
+
+ def mock_test_connection(self):
+ captured_extra["value"] = self.extra
+ return True, "mocked"
+
+ body = {"connection_id": TEST_CONN_ID, "conn_type": "fs", "extra":
'{"path": "***"}'}
+ with mock.patch.object(Connection, "test_connection",
mock_test_connection):
+ response = test_client.post("/connections/test", json=body)
+
+ assert response.status_code == 200
+ assert json.loads(captured_extra["value"]) == {"path": "/secret"}
+
+ @mock.patch(
+
"airflow.api_fastapi.core_api.datamodels.connections._sensitive_extra_keys",
+ autospec=True,
+ return_value=frozenset({"path", "extra__fs__path"}),
+ )
+ @mock.patch.dict(os.environ, {"AIRFLOW__CORE__TEST_CONNECTION": "Enabled"})
+ def test_should_not_reuse_provider_sensitive_extra_for_another_type(self,
_, test_client, session):
+ session.add(Connection(conn_id=TEST_CONN_ID, conn_type="fs",
extra='{"path": "/secret"}'))
+ session.commit()
+ captured_extra = {}
+
+ def mock_test_connection(self):
+ captured_extra["value"] = self.extra
+ return True, "mocked"
+
+ body = {"connection_id": TEST_CONN_ID, "conn_type": "sqlite", "extra":
'{"path": "***"}'}
+ with mock.patch.object(Connection, "test_connection",
mock_test_connection):
+ response = test_client.post("/connections/test", json=body)
+
+ assert response.status_code == 200
+ assert json.loads(captured_extra["value"]) == {"path": "***"}
+
@mock.patch.dict(os.environ, {"AIRFLOW__CORE__TEST_CONNECTION": "Enabled"})
def test_should_merge_both_password_and_extra(self, test_client, session):
connection = Connection(
