From aad1a4eaecb95da6763a5461cb0b41b544271a06 Mon Sep 17 00:00:00 2001
From: Pierre Jeambrun <pierrejbrun@gmail.com>
Date: Tue, 29 Jul 2025 16:51:59 +0200
Subject: [PATCH 1/2] Revert "Fix editing connection with sensitive extra field
 (#52403)"

This reverts commit 8ef792dafef6e64faf5daa376bfa5238b1c14c87.
---
 .../core_api/datamodels/connections.py        | 25 +++++-
 .../src/pages/Connections/ConnectionForm.tsx  | 12 +--
 .../ui/src/queries/useEditConnection.tsx      |  4 +-
 .../routes/public/test_connections.py         | 81 ++++++++++++++++---
 4 files changed, 96 insertions(+), 26 deletions(-)

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 f2ac1f7a9403a..2a6bd6bec8af2 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
@@ -17,12 +17,15 @@
 
 from __future__ import annotations
 
+import json
 from collections import abc
 from typing import Annotated
 
-from pydantic import Field
+from pydantic import Field, field_validator
+from pydantic_core.core_schema import ValidationInfo
 
 from airflow.api_fastapi.core_api.base import BaseModel, StrictBaseModel
+from airflow.sdk.execution_time.secrets_masker import redact
 
 
 # Response Models
@@ -39,6 +42,26 @@ class ConnectionResponse(BaseModel):
     password: str | None
     extra: str | None
 
+    @field_validator("password", mode="after")
+    @classmethod
+    def redact_password(cls, v: str | None, field_info: ValidationInfo) -> str | None:
+        if v is None:
+            return None
+        return str(redact(v, field_info.field_name))
+
+    @field_validator("extra", mode="before")
+    @classmethod
+    def redact_extra(cls, v: str | None) -> str | None:
+        if v is None:
+            return None
+        try:
+            extra_dict = json.loads(v)
+            redacted_dict = redact(extra_dict)
+            return json.dumps(redacted_dict)
+        except json.JSONDecodeError:
+            # we can't redact fields in an unstructured `extra`
+            return v
+
 
 class ConnectionCollectionResponse(BaseModel):
     """Connection Collection serializer for responses."""
diff --git a/airflow-core/src/airflow/ui/src/pages/Connections/ConnectionForm.tsx b/airflow-core/src/airflow/ui/src/pages/Connections/ConnectionForm.tsx
index 9e4e38647eaf4..bf921a1de3c99 100644
--- a/airflow-core/src/airflow/ui/src/pages/Connections/ConnectionForm.tsx
+++ b/airflow-core/src/airflow/ui/src/pages/Connections/ConnectionForm.tsx
@@ -57,7 +57,7 @@ const ConnectionForm = ({
   const { conf: extra, setConf } = useParamStore();
   const {
     control,
-    formState: { isDirty, isValid },
+    formState: { isValid },
     handleSubmit,
     reset,
     watch,
@@ -94,14 +94,6 @@ const ConnectionForm = ({
     mutateConnection(data);
   };
 
-  const hasChanges = () => {
-    if (isDirty) {
-      return true;
-    }
-
-    return JSON.stringify(JSON.parse(extra)) !== JSON.stringify(JSON.parse(initialConnection.extra));
-  };
-
   const validateAndPrettifyJson = (value: string) => {
     try {
       const parsedJson = JSON.parse(value) as JSON;
@@ -242,7 +234,7 @@ const ConnectionForm = ({
           <Spacer />
           <Button
             colorPalette="blue"
-            disabled={Boolean(errors.conf) || formErrors || isPending || !isValid || !hasChanges()}
+            disabled={Boolean(errors.conf) || formErrors || isPending || !isValid}
             onClick={() => void handleSubmit(onSubmit)()}
           >
             <FiSave /> {translate("formActions.save")}
diff --git a/airflow-core/src/airflow/ui/src/queries/useEditConnection.tsx b/airflow-core/src/airflow/ui/src/queries/useEditConnection.tsx
index 2392eda11d656..60c813aa644dd 100644
--- a/airflow-core/src/airflow/ui/src/queries/useEditConnection.tsx
+++ b/airflow-core/src/airflow/ui/src/queries/useEditConnection.tsx
@@ -65,9 +65,7 @@ export const useEditConnection = (
   const editConnection = (requestBody: ConnectionBody) => {
     const updateMask: Array<string> = [];
 
-    if (
-      JSON.stringify(JSON.parse(requestBody.extra)) !== JSON.stringify(JSON.parse(initialConnection.extra))
-    ) {
+    if (requestBody.extra !== initialConnection.extra) {
       updateMask.push("extra");
     }
     if (requestBody.conn_type !== initialConnection.conn_type) {
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 eba9f6ddaf83e..6c56232a68a92 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
@@ -159,6 +159,35 @@ def test_get_should_respond_200_with_extra(self, test_client, session):
         assert body["conn_type"] == TEST_CONN_TYPE
         assert body["extra"] == '{"extra_key": "extra_value"}'
 
+    @pytest.mark.enable_redact
+    def test_get_should_respond_200_with_extra_redacted(self, test_client, session):
+        self.create_connection()
+        connection = session.query(Connection).first()
+        connection.extra = '{"password": "test-password"}'
+        session.commit()
+        response = test_client.get(f"/connections/{TEST_CONN_ID}")
+        assert response.status_code == 200
+        body = response.json()
+        assert body["connection_id"] == TEST_CONN_ID
+        assert body["conn_type"] == TEST_CONN_TYPE
+        assert body["extra"] == '{"password": "***"}'
+
+    @pytest.mark.enable_redact
+    def test_get_should_not_overmask_short_password_value_in_extra(self, test_client, session):
+        connection = Connection(
+            conn_id=TEST_CONN_ID, conn_type="generic", login="a", password="a", extra='{"key": "value"}'
+        )
+        session.add(connection)
+        session.commit()
+
+        response = test_client.get(f"/connections/{TEST_CONN_ID}")
+        assert response.status_code == 200
+        body = response.json()
+        assert body["connection_id"] == TEST_CONN_ID
+        assert body["conn_type"] == "generic"
+        assert body["login"] == "a"
+        assert body["extra"] == '{"key": "value"}'
+
 
 class TestGetConnections(TestConnectionEndpoint):
     @pytest.mark.parametrize(
@@ -280,6 +309,7 @@ def test_post_should_respond_already_exist(self, test_client, body):
         assert "detail" in response_json
         assert list(response_json["detail"].keys()) == ["reason", "statement", "orig_error", "message"]
 
+    @pytest.mark.enable_redact
     @pytest.mark.parametrize(
         "body, expected_response",
         [
@@ -292,7 +322,21 @@ def test_post_should_respond_already_exist(self, test_client, body):
                     "extra": None,
                     "host": None,
                     "login": None,
-                    "password": "test-password",
+                    "password": "***",
+                    "port": None,
+                    "schema": None,
+                },
+            ),
+            (
+                {"connection_id": TEST_CONN_ID, "conn_type": TEST_CONN_TYPE, "password": "?>@#+!_%()#"},
+                {
+                    "connection_id": TEST_CONN_ID,
+                    "conn_type": TEST_CONN_TYPE,
+                    "description": None,
+                    "extra": None,
+                    "host": None,
+                    "login": None,
+                    "password": "***",
                     "port": None,
                     "schema": None,
                 },
@@ -308,23 +352,21 @@ def test_post_should_respond_already_exist(self, test_client, body):
                     "connection_id": TEST_CONN_ID,
                     "conn_type": TEST_CONN_TYPE,
                     "description": None,
-                    "extra": '{"password": "test-password"}',
+                    "extra": '{"password": "***"}',
                     "host": None,
                     "login": None,
-                    "password": "A!rF|0wi$aw3s0m3",
+                    "password": "***",
                     "port": None,
                     "schema": None,
                 },
             ),
         ],
     )
-    def test_post_should_response_201_password_not_masked(
-        self, test_client, body, expected_response, session
-    ):
+    def test_post_should_response_201_redacted_password(self, test_client, body, expected_response, session):
         response = test_client.post("/connections", json=body)
         assert response.status_code == 201
         assert response.json() == expected_response
-        _check_last_log(session, dag_id=None, event="post_connection", logical_date=None)
+        _check_last_log(session, dag_id=None, event="post_connection", logical_date=None, check_masked=True)
 
 
 class TestPatchConnection(TestConnectionEndpoint):
@@ -734,7 +776,22 @@ def test_patch_should_respond_404(self, test_client, body):
                     "extra": None,
                     "host": "some_host_a",
                     "login": "some_login",
-                    "password": "test-password",
+                    "password": "***",
+                    "port": 8080,
+                    "schema": None,
+                },
+                {"update_mask": ["password"]},
+            ),
+            (
+                {"connection_id": TEST_CONN_ID, "conn_type": TEST_CONN_TYPE, "password": "?>@#+!_%()#"},
+                {
+                    "connection_id": TEST_CONN_ID,
+                    "conn_type": TEST_CONN_TYPE,
+                    "description": "some_description_a",
+                    "extra": None,
+                    "host": "some_host_a",
+                    "login": "some_login",
+                    "password": "***",
                     "port": 8080,
                     "schema": None,
                 },
@@ -751,10 +808,10 @@ def test_patch_should_respond_404(self, test_client, body):
                     "connection_id": TEST_CONN_ID,
                     "conn_type": TEST_CONN_TYPE,
                     "description": "some_description_a",
-                    "extra": '{"password": "test-password"}',
+                    "extra": '{"password": "***"}',
                     "host": "some_host_a",
                     "login": "some_login",
-                    "password": "A!rF|0wi$aw3s0m3",
+                    "password": "***",
                     "port": 8080,
                     "schema": None,
                 },
@@ -762,14 +819,14 @@ def test_patch_should_respond_404(self, test_client, body):
             ),
         ],
     )
-    def test_patch_should_response_200_password_not_masked(
+    def test_patch_should_response_200_redacted_password(
         self, test_client, session, body, expected_response, update_mask
     ):
         self.create_connections()
         response = test_client.patch(f"/connections/{TEST_CONN_ID}", json=body, params=update_mask)
         assert response.status_code == 200
         assert response.json() == expected_response
-        _check_last_log(session, dag_id=None, event="patch_connection", logical_date=None)
+        _check_last_log(session, dag_id=None, event="patch_connection", logical_date=None, check_masked=True)
 
 
 class TestConnection(TestConnectionEndpoint):

From d92032a9311ba973d875f9a21857a2a148693e58 Mon Sep 17 00:00:00 2001
From: pierrejeambrun <pierrejbrun@gmail.com>
Date: Tue, 29 Jul 2025 17:30:57 +0200
Subject: [PATCH 2/2] Fix CI

---
 .pre-commit-config.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 8bb7dd9611909..2f144e403de36 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1635,6 +1635,7 @@ repos:
           ^airflow-core/src/airflow/models/__init__\.py$|
           ^airflow-core/src/airflow/api/common/mark_tasks\.py$|
           ^airflow-core/src/airflow/api_fastapi/core_api/datamodels/assets\.py$|
+          ^airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections\.py$|
           ^airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl\.py$|
           ^airflow-core/src/airflow/api_fastapi/core_api/datamodels/variables\.py$|
           ^airflow-core/src/airflow/api_fastapi/core_api/routes/ui/grid\.py$|