You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by bu...@apache.org on 2025/11/09 17:36:59 UTC

(airflow) branch revert-57988-bugfix/remove-unused-null-fernet created (now cb56ad7a889)

This is an automated email from the ASF dual-hosted git repository.

bugraoz pushed a change to branch revert-57988-bugfix/remove-unused-null-fernet
in repository https://gitbox.apache.org/repos/asf/airflow.git


      at cb56ad7a889 Revert "Remove unused NullFernet from Crypto (#57988)"

This branch includes the following new commits:

     new cb56ad7a889 Revert "Remove unused NullFernet from Crypto (#57988)"

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(airflow) 01/01: Revert "Remove unused NullFernet from Crypto (#57988)"

Posted by bu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

bugraoz pushed a commit to branch revert-57988-bugfix/remove-unused-null-fernet
in repository https://gitbox.apache.org/repos/asf/airflow.git

commit cb56ad7a8893051a6086cda23ce79e422f5e17da
Author: Bugra Ozturk <bu...@users.noreply.github.com>
AuthorDate: Sun Nov 9 18:36:48 2025 +0100

    Revert "Remove unused NullFernet from Crypto (#57988)"
    
    This reverts commit c47c0817d8e11f885f4eef91232c08e1469e8c95.
---
 airflow-core/src/airflow/models/connection.py      | 14 ++++-
 airflow-core/src/airflow/models/crypto.py          | 61 ++++++++++++++++++++--
 airflow-core/src/airflow/models/variable.py        |  2 +-
 airflow-core/tests/unit/always/test_connection.py  | 12 +++++
 .../cli/commands/test_rotate_fernet_key_command.py | 24 +++++++++
 airflow-core/tests/unit/models/test_variable.py    | 14 +++++
 6 files changed, 121 insertions(+), 6 deletions(-)

diff --git a/airflow-core/src/airflow/models/connection.py b/airflow-core/src/airflow/models/connection.py
index 3f9e5e3deed..f64c38b5efe 100644
--- a/airflow-core/src/airflow/models/connection.py
+++ b/airflow-core/src/airflow/models/connection.py
@@ -343,6 +343,11 @@ class Connection(Base, LoggingMixin):
         """Return encrypted password."""
         if self._password and self.is_encrypted:
             fernet = get_fernet()
+            if not fernet.is_encrypted:
+                raise AirflowException(
+                    f"Can't decrypt encrypted password for login={self.login}  "
+                    f"FERNET_KEY configuration is missing"
+                )
             return fernet.decrypt(bytes(self._password, "utf-8")).decode()
         return self._password
 
@@ -351,7 +356,7 @@ class Connection(Base, LoggingMixin):
         if value:
             fernet = get_fernet()
             self._password = fernet.encrypt(bytes(value, "utf-8")).decode()
-            self.is_encrypted = True
+            self.is_encrypted = fernet.is_encrypted
 
     @declared_attr
     def password(cls):
@@ -362,6 +367,11 @@ class Connection(Base, LoggingMixin):
         """Return encrypted extra-data."""
         if self._extra and self.is_extra_encrypted:
             fernet = get_fernet()
+            if not fernet.is_encrypted:
+                raise AirflowException(
+                    f"Can't decrypt `extra` params for login={self.login}, "
+                    f"FERNET_KEY configuration is missing"
+                )
             extra_val = fernet.decrypt(bytes(self._extra, "utf-8")).decode()
         else:
             extra_val = self._extra
@@ -375,7 +385,7 @@ class Connection(Base, LoggingMixin):
             self._validate_extra(value, self.conn_id)
             fernet = get_fernet()
             self._extra = fernet.encrypt(bytes(value, "utf-8")).decode()
-            self.is_extra_encrypted = True
+            self.is_extra_encrypted = fernet.is_encrypted
         else:
             self._extra = value
             self.is_extra_encrypted = False
diff --git a/airflow-core/src/airflow/models/crypto.py b/airflow-core/src/airflow/models/crypto.py
index 31423bb8444..c62446b7631 100644
--- a/airflow-core/src/airflow/models/crypto.py
+++ b/airflow-core/src/airflow/models/crypto.py
@@ -30,6 +30,8 @@ log = logging.getLogger(__name__)
 class FernetProtocol(Protocol):
     """This class is only used for TypeChecking (for IDEs, mypy, etc)."""
 
+    is_encrypted: bool
+
     def decrypt(self, msg: bytes | str, ttl: int | None = None) -> bytes:
         """Decrypt with Fernet."""
         ...
@@ -38,9 +40,57 @@ class FernetProtocol(Protocol):
         """Encrypt with Fernet."""
         ...
 
+
+class _NullFernet:
+    """
+    A "Null" encryptor class that doesn't encrypt or decrypt but that presents a similar interface to Fernet.
+
+    The purpose of this is to make the rest of the code not have to know the
+    difference, and to only display the message once, not 20 times when
+    `airflow db migrate` is run.
+    """
+
+    is_encrypted = False
+
+    def decrypt(self, msg: bytes | str, ttl: int | None = None) -> bytes:
+        """Decrypt with Fernet."""
+        if isinstance(msg, bytes):
+            return msg
+        if isinstance(msg, str):
+            return msg.encode("utf-8")
+        raise ValueError(f"Expected bytes or str, got {type(msg)}")
+
+    def encrypt(self, msg: bytes) -> bytes:
+        """Encrypt with Fernet."""
+        return msg
+
+
+class _RealFernet:
+    """
+    A wrapper around the real Fernet to set is_encrypted to True.
+
+    This class is only used internally to avoid changing the interface of
+    the get_fernet function.
+    """
+
+    from cryptography.fernet import Fernet, MultiFernet
+
+    is_encrypted = True
+
+    def __init__(self, fernet: MultiFernet):
+        self._fernet = fernet
+
+    def decrypt(self, msg: bytes | str, ttl: int | None = None) -> bytes:
+        """Decrypt with Fernet."""
+        return self._fernet.decrypt(msg, ttl)
+
+    def encrypt(self, msg: bytes) -> bytes:
+        """Encrypt with Fernet."""
+        return self._fernet.encrypt(msg)
+
     def rotate(self, msg: bytes | str) -> bytes:
         """Rotate the Fernet key for the given message."""
-        ...
+        return self._fernet.rotate(msg)
 
 
 @cache
@@ -57,7 +107,12 @@ def get_fernet() -> FernetProtocol:
     from cryptography.fernet import Fernet, MultiFernet
 
     try:
-        fernet_key = conf.get_mandatory_value("core", "FERNET_KEY")
-        return MultiFernet([Fernet(fernet_part.encode("utf-8")) for fernet_part in fernet_key.split(",")])
+        fernet_key = conf.get("core", "FERNET_KEY")
+        if not fernet_key:
+            log.warning("empty cryptography key - values will not be stored encrypted.")
+            return _NullFernet()
+
+        fernet = MultiFernet([Fernet(fernet_part.encode("utf-8")) for fernet_part in fernet_key.split(",")])
+        return _RealFernet(fernet)
     except (ValueError, TypeError) as value_error:
         raise AirflowException(f"Could not create Fernet object: {value_error}")
diff --git a/airflow-core/src/airflow/models/variable.py b/airflow-core/src/airflow/models/variable.py
index c5037561cbc..a13eb4fe158 100644
--- a/airflow-core/src/airflow/models/variable.py
+++ b/airflow-core/src/airflow/models/variable.py
@@ -97,7 +97,7 @@ class Variable(Base, LoggingMixin):
         if value is not None:
             fernet = get_fernet()
             self._val = fernet.encrypt(bytes(value, "utf-8")).decode()
-            self.is_encrypted = True
+            self.is_encrypted = fernet.is_encrypted
 
     @declared_attr
     def val(cls):
diff --git a/airflow-core/tests/unit/always/test_connection.py b/airflow-core/tests/unit/always/test_connection.py
index 3f3b3af4bfa..d613c0d77da 100644
--- a/airflow-core/tests/unit/always/test_connection.py
+++ b/airflow-core/tests/unit/always/test_connection.py
@@ -109,6 +109,18 @@ class TestConnection:
     def teardown_method(self):
         self.patcher.stop()
 
+    @conf_vars({("core", "fernet_key"): ""})
+    def test_connection_extra_no_encryption(self):
+        """
+        Tests extras on a new connection without encryption. The fernet key
+        is set to a non-base64-encoded string and the extra is stored without
+        encryption.
+        """
+        crypto.get_fernet.cache_clear()
+        test_connection = Connection(extra='{"apache": "airflow"}')
+        assert not test_connection.is_extra_encrypted
+        assert test_connection.extra == '{"apache": "airflow"}'
+
     @conf_vars({("core", "fernet_key"): Fernet.generate_key().decode()})
     def test_connection_extra_with_encryption(self):
         """
diff --git a/airflow-core/tests/unit/cli/commands/test_rotate_fernet_key_command.py b/airflow-core/tests/unit/cli/commands/test_rotate_fernet_key_command.py
index b2638208fa3..dbd250c2c2d 100644
--- a/airflow-core/tests/unit/cli/commands/test_rotate_fernet_key_command.py
+++ b/airflow-core/tests/unit/cli/commands/test_rotate_fernet_key_command.py
@@ -49,8 +49,14 @@ class TestRotateFernetKeyCommand:
     def test_should_rotate_variable(self, session):
         fernet_key1 = Fernet.generate_key()
         fernet_key2 = Fernet.generate_key()
+        var1_key = f"{__file__}_var1"
         var2_key = f"{__file__}_var2"
 
+        # Create unencrypted variable
+        with conf_vars({("core", "fernet_key"): ""}):
+            get_fernet.cache_clear()  # Clear cached fernet
+            Variable.set(key=var1_key, value="value")
+
         # Create encrypted variable
         with conf_vars({("core", "fernet_key"): fernet_key1.decode()}):
             get_fernet.cache_clear()  # Clear cached fernet
@@ -65,14 +71,25 @@ class TestRotateFernetKeyCommand:
         # Assert correctness using a new fernet key
         with conf_vars({("core", "fernet_key"): fernet_key2.decode()}):
             get_fernet.cache_clear()  # Clear cached fernet
+            var1 = session.query(Variable).filter(Variable.key == var1_key).first()
+            # Unencrypted variable should be unchanged
+            assert Variable.get(key=var1_key) == "value"
+            assert var1._val == "value"
             assert Variable.get(key=var2_key) == "value"
 
     @provide_session
     def test_should_rotate_connection(self, session, mock_supervisor_comms):
         fernet_key1 = Fernet.generate_key()
         fernet_key2 = Fernet.generate_key()
+        var1_key = f"{__file__}_var1"
         var2_key = f"{__file__}_var2"
 
+        # Create unencrypted variable
+        with conf_vars({("core", "fernet_key"): ""}):
+            get_fernet.cache_clear()  # Clear cached fernet
+            session.add(Connection(conn_id=var1_key, uri="mysql://user:pass@localhost"))
+            session.commit()
+
         # Create encrypted variable
         with conf_vars({("core", "fernet_key"): fernet_key1.decode()}):
             get_fernet.cache_clear()  # Clear cached fernet
@@ -102,10 +119,17 @@ class TestRotateFernetKeyCommand:
                 )
             raise Exception(f"Connection {conn_id} not found")
 
+        # Mock the send method to return our connection data
+        mock_supervisor_comms.send.return_value = mock_get_connection(var1_key)
+
         # Assert correctness using a new fernet key
         with conf_vars({("core", "fernet_key"): fernet_key2.decode()}):
             get_fernet.cache_clear()  # Clear cached fernet
 
+            # Unencrypted variable should be unchanged
+            conn1: Connection = BaseHook.get_connection(var1_key)
+            assert conn1.password == "pass"
+
             # Mock for the second connection
             mock_supervisor_comms.send.return_value = mock_get_connection(var2_key)
             assert BaseHook.get_connection(var2_key).password == "pass"
diff --git a/airflow-core/tests/unit/models/test_variable.py b/airflow-core/tests/unit/models/test_variable.py
index 6ae364be503..b02a760a665 100644
--- a/airflow-core/tests/unit/models/test_variable.py
+++ b/airflow-core/tests/unit/models/test_variable.py
@@ -52,6 +52,20 @@ class TestVariable:
             yield
         db.clear_db_variables()
 
+    @conf_vars({("core", "fernet_key"): "", ("core", "unit_test_mode"): "True"})
+    def test_variable_no_encryption(self, session):
+        """
+        Test variables without encryption
+        """
+        crypto.get_fernet.cache_clear()
+        Variable.set(key="key", value="value", session=session)
+        test_var = session.query(Variable).filter(Variable.key == "key").one()
+        assert not test_var.is_encrypted
+        assert test_var.val == "value"
+        # We always call mask_secret for variables, and let the SecretsMasker decide based on the name if it
+        # should mask anything. That logic is tested in test_secrets_masker.py
+        self.mask_secret.assert_called_once_with("value", "key")
+
     @conf_vars({("core", "fernet_key"): Fernet.generate_key().decode()})
     def test_variable_with_encryption(self, session):
         """