Skip to content

ENG-2857 Use callable for encryption key #7588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erosselli merged 8 commits into from
Mar 9, 2026
Merged

ENG-2857 Use callable for encryption key #7588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
eaf7142
Use callable for encryption key
erosselli Mar 6, 2026
e484e26
static checks
erosselli Mar 9, 2026
b0a36e0
Add changelog
erosselli Mar 9, 2026
2270927
fix test
erosselli Mar 9, 2026
eebec4b
Remove references to DEK as we have not implemented envelope encrypti…
erosselli Mar 9, 2026
007e68e
add warning log if encryption key is empty
erosselli Mar 9, 2026
df7a648
better check
erosselli Mar 9, 2026
8cadc03
Merge branch 'main' into erosselli/2857
erosselli Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/7588-encrypted-type-factory.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type: Changed
description: Centralized encryption column definitions behind an encrypted_type() factory with callable key
pr: 7588
labels: []
53 changes: 53 additions & 0 deletions src/fides/api/db/encryption_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
from typing import Union

from loguru import logger
from sqlalchemy import Text
from sqlalchemy.types import TypeEngine
from sqlalchemy_utils.types.encrypted.encrypted_type import (
AesGcmEngine,
StringEncryptedType,
)

from fides.config import CONFIG

_cached_dek: str | None = None


def get_encryption_key() -> str:
"""
Return the encryption key set in CONFIG.security.app_encryption_key.
Cached for the lifetime of the process (called on every encrypt/decrypt).

TODO: implement support for other key managers like AWS KMS
Copy link
Copy Markdown
Contributor

@JadeCara JadeCara Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everytime I see a TODO (sorry) - I have to ask, do we have this ticketed?

Copy link
Copy Markdown
Contributor Author

@erosselli erosselli Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we do! we have a whole epic for the encryption changes :)

"""
global _cached_dek
if _cached_dek is not None:
return _cached_dek

_cached_dek = CONFIG.security.app_encryption_key
if not _cached_dek:
logger.warning(
"App encryption key is empty, this may lead to unexpected issues"
)

return _cached_dek
Comment thread
erosselli marked this conversation as resolved.
Comment on lines +23 to +33
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Cache is ineffective and warning is skipped when encryption key is None.

If CONFIG.security.app_encryption_key returns None:

  1. The is not None check on line 24 passes (because _cached_dek starts as None)
  2. _cached_dek gets assigned None from config
  3. The warning check == "" doesn't match None
  4. On subsequent calls, line 24's check fails again since None is not None is False

This causes repeated config reads and no warning for a None key. Consider using a sentinel value to distinguish "not yet cached" from "cached as None/empty".

🔧 Proposed fix using a sentinel value 
+_NOT_SET = object()  # sentinel to distinguish "not cached" from "cached as None"
-_cached_dek: str | None = None
+_cached_dek: str | None | object = _NOT_SET


 def get_encryption_key() -> str:
     """
     Return the encryption key set in CONFIG.security.app_encryption_key.
     Cached for the lifetime of the process (called on every encrypt/decrypt).

     TODO: implement support for other key managers like AWS KMS
     """
     global _cached_dek
-    if _cached_dek is not None:
+    if _cached_dek is not _NOT_SET:
         return _cached_dek

     _cached_dek = CONFIG.security.app_encryption_key
-    if _cached_dek == "":
+    if not _cached_dek:  # catches both None and empty string
         logger.warning(
             "App encryption key is empty, this may lead to unexpected issues"
         )

     return _cached_dek
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/fides/api/db/encryption_utils.py` around lines 23 - 33, The caching logic
in encryption_utils.py incorrectly conflates "not cached yet" with a cached
value of None/empty; change the sentinel for _cached_dek to a unique object
(e.g., _UNSET = object()) and initialize _cached_dek = _UNSET, then in the
getter check "if _cached_dek is not _UNSET: return _cached_dek", set _cached_dek
= CONFIG.security.app_encryption_key (keeping the global declaration), and
update the warning to trigger when the cached value is falsy (e.g., if
_cached_dek in (None, "") or if not _cached_dek) so a None config is cached once
and the warning is emitted.



def _reset_encryption_key_cache() -> None:
"""Reset the cached encryption key. For testing only."""
global _cached_dek
_cached_dek = None
Comment on lines +13 to +39
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't freeze the encryption key after first use.

get_encryption_key() caches CONFIG.security.app_encryption_key in _cached_dek, so after the first encrypt/decrypt in a worker the key is no longer resolved at use time. That undermines the callable-key goal of this refactor and also changes the new direct callers in src/fides/api/util/encryption/aes_gcm_encryption_util.py and src/fides/api/models/privacy_request/webhook.py: a mid-process key override/rotation will keep using the stale value until restart. Please either remove the cache here or scope the cached path to the ORM type factory only.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/fides/api/db/encryption_utils.py` around lines 12 - 28, The current
implementation freezes the DEK by caching CONFIG.security.app_encryption_key in
the module-level _cached_dek inside get_encryption_key(), causing mid-process
rotations to be ignored; remove the global caching so get_encryption_key()
always resolves and returns CONFIG.security.app_encryption_key at call time (and
delete or repurpose _cached_dek and _reset_encryption_key_cache), or
alternatively move caching responsibility into the ORM type factory only (so
only that factory uses a local cached value) while keeping get_encryption_key()
uncached for direct callers like aes_gcm_encryption_util.py and webhook.py.



def encrypted_type(
type_in: Union[type[TypeEngine], TypeEngine, None] = None,
) -> StringEncryptedType:
"""Build a StringEncryptedType with a callable key."""
if type_in is None:
type_in = Text()
return StringEncryptedType(
type_in=type_in,
key=get_encryption_key, # callable, NOT called
engine=AesGcmEngine,
padding="pkcs5",
)
13 changes: 2 additions & 11 deletions src/fides/api/db/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,9 @@

from sqlalchemy import Text
from sqlalchemy.types import TypeEngine
from sqlalchemy_utils.types.encrypted.encrypted_type import (
AesGcmEngine,
StringEncryptedType,
)

from fides.api.db.encryption_utils import encrypted_type
from fides.api.util.custom_json_encoder import CustomJSONEncoder, _custom_decoder
from fides.config import CONFIG

if TYPE_CHECKING:
# Override the Enum constructor not to accept a list of strings and confuse mypy:
Expand Down Expand Up @@ -63,10 +59,5 @@ def optionally_encrypted_type(
type_in = Text()

if encryption_enabled:
return StringEncryptedType(
type_in=type_in,
key=CONFIG.security.app_encryption_key,
engine=AesGcmEngine,
padding="pkcs5",
)
return encrypted_type(type_in=type_in)
return type_in
25 changes: 4 additions & 21 deletions src/fides/api/models/application_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,10 @@
from sqlalchemy import Boolean, CheckConstraint, Column
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy.orm import Session
from sqlalchemy_utils.types.encrypted.encrypted_type import (
AesGcmEngine,
StringEncryptedType,
)

from fides.api.db.base_class import Base, JSONTypeOverride
from fides.config import CONFIG, FidesConfig
from fides.api.db.encryption_utils import encrypted_type
from fides.config import FidesConfig


class ApplicationConfig(Base):
Expand All @@ -26,26 +23,12 @@ class ApplicationConfig(Base):
"""

api_set = Column(
MutableDict.as_mutable(
StringEncryptedType(
JSONTypeOverride,
CONFIG.security.app_encryption_key,
AesGcmEngine,
"pkcs5",
)
),
MutableDict.as_mutable(encrypted_type(type_in=JSONTypeOverride)),
nullable=False,
default={},
) # store as encrypted JSON blob since config settings may have sensitive data
config_set = Column(
MutableDict.as_mutable(
StringEncryptedType(
JSONTypeOverride,
CONFIG.security.app_encryption_key,
AesGcmEngine,
"pkcs5",
)
),
MutableDict.as_mutable(encrypted_type(type_in=JSONTypeOverride)),
nullable=False,
default={},
) # store as encrypted JSON blob since config settings may have sensitive data
Expand Down
27 changes: 4 additions & 23 deletions src/fides/api/models/chat_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,9 @@

from sqlalchemy import Boolean, Column, String, UniqueConstraint
from sqlalchemy.ext.declarative import declared_attr
from sqlalchemy_utils.types.encrypted.encrypted_type import (
AesGcmEngine,
StringEncryptedType,
)

from fides.api.db.base_class import Base
from fides.config import CONFIG
from fides.api.db.encryption_utils import encrypted_type


class ChatConfig(Base):
Expand All @@ -27,30 +23,15 @@ def __tablename__(self) -> str:
workspace_url = Column(String, nullable=True)
client_id = Column(String, nullable=True)
client_secret = Column(
StringEncryptedType(
type_in=String(),
key=CONFIG.security.app_encryption_key,
engine=AesGcmEngine,
padding="pkcs5",
),
encrypted_type(type_in=String()),
nullable=True,
)
access_token = Column(
StringEncryptedType(
type_in=String(),
key=CONFIG.security.app_encryption_key,
engine=AesGcmEngine,
padding="pkcs5",
),
encrypted_type(type_in=String()),
nullable=True,
)
signing_secret = Column(
StringEncryptedType(
type_in=String(),
key=CONFIG.security.app_encryption_key,
engine=AesGcmEngine,
padding="pkcs5",
),
encrypted_type(type_in=String()),
nullable=True,
)
enabled = Column(Boolean, nullable=False, default=False)
Expand Down
13 changes: 2 additions & 11 deletions src/fides/api/models/connection_oauth_credentials.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@
from sqlalchemy import Column, Enum, ForeignKey, String
from sqlalchemy.ext.declarative import declared_attr
from sqlalchemy.orm import relationship
from sqlalchemy_utils.types.encrypted.encrypted_type import (
AesGcmEngine,
StringEncryptedType,
)

from fides.api.db.base_class import Base
from fides.api.db.encryption_utils import encrypted_type
from fides.api.models.connectionconfig import ConnectionConfig

# pylint: disable=unused-import
from fides.api.models.sql_models import System # type: ignore[attr-defined]
from fides.config import CONFIG


class OAuthGrantType(enum.Enum):
Expand Down Expand Up @@ -42,12 +38,7 @@ def __tablename__(cls) -> str:
scope = Column(String, nullable=True)
client_id = Column(String, nullable=False)
client_secret = Column(
StringEncryptedType(
String,
CONFIG.security.app_encryption_key,
AesGcmEngine,
"pkcs5",
),
encrypted_type(type_in=String()),
nullable=False,
)

Expand Down
15 changes: 2 additions & 13 deletions src/fides/api/models/connectionconfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,14 @@
from sqlalchemy.dialects.postgresql import ARRAY, JSONB
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy.orm import RelationshipProperty, Session, relationship
from sqlalchemy_utils.types.encrypted.encrypted_type import (
AesGcmEngine,
StringEncryptedType,
)

from fides.api.common_exceptions import KeyOrNameAlreadyExists
from fides.api.db.base_class import Base, FidesBase, JSONTypeOverride
from fides.api.db.encryption_utils import encrypted_type
from fides.api.models.consent_automation import ConsentAutomation
from fides.api.models.sql_models import System # type: ignore[attr-defined]
from fides.api.schemas.policy import ActionType
from fides.api.schemas.saas.saas_config import SaaSConfig
from fides.config import CONFIG

if TYPE_CHECKING:
from fides.api.models.detection_discovery.core import MonitorConfig
Expand Down Expand Up @@ -193,14 +189,7 @@ class ConnectionConfig(Base):
connection_type = Column(Enum(ConnectionType), nullable=False)
access = Column(Enum(AccessLevel), nullable=False)
secrets = Column(
MutableDict.as_mutable(
StringEncryptedType(
JSONTypeOverride,
CONFIG.security.app_encryption_key,
AesGcmEngine,
"pkcs5",
)
),
MutableDict.as_mutable(encrypted_type(type_in=JSONTypeOverride)),
nullable=True,
) # Type bytes in the db
last_test_timestamp = Column(DateTime(timezone=True))
Expand Down
13 changes: 2 additions & 11 deletions src/fides/api/models/fides_user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,18 @@
from sqlalchemy import Boolean, Column, DateTime, String
from sqlalchemy import Enum as EnumColumn
from sqlalchemy.orm import Session, relationship
from sqlalchemy_utils.types.encrypted.encrypted_type import (
AesGcmEngine,
StringEncryptedType,
)

from fides.api.common_exceptions import SystemManagerException
from fides.api.cryptography.cryptographic_util import (
generate_salt,
hash_credential_with_salt,
)
from fides.api.db.base_class import Base
from fides.api.db.encryption_utils import encrypted_type
from fides.api.models.audit_log import AuditLog

# Intentionally importing SystemManager here to build the FidesUser.systems relationship
from fides.api.schemas.user import DisabledReason
from fides.config import CONFIG

if TYPE_CHECKING:
from fides.api.models.detection_discovery import MonitorConfig
Expand Down Expand Up @@ -51,12 +47,7 @@ class FidesUser(Base):
password_reset_at = Column(DateTime(timezone=True), nullable=True)
password_login_enabled = Column(Boolean, nullable=True)
totp_secret = Column(
StringEncryptedType(
type_in=String(),
key=CONFIG.security.app_encryption_key,
engine=AesGcmEngine,
padding="pkcs5",
),
encrypted_type(type_in=String()),
nullable=True,
)

Expand Down
15 changes: 2 additions & 13 deletions src/fides/api/models/identity_salt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
from sqlalchemy import Boolean, CheckConstraint, Column, UniqueConstraint
from sqlalchemy.ext.declarative import declared_attr
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy_utils.types.encrypted.encrypted_type import (
AesGcmEngine,
StringEncryptedType,
)

from fides.api.db.base_class import (
Base, # type: ignore[attr-defined]
JSONTypeOverride,
)
from fides.config import CONFIG
from fides.api.db.encryption_utils import encrypted_type


class IdentitySalt(Base):
Expand All @@ -23,14 +19,7 @@ def __tablename__(self) -> str:
return "identity_salt"

encrypted_value = Column(
MutableDict.as_mutable(
StringEncryptedType(
JSONTypeOverride,
CONFIG.security.app_encryption_key,
AesGcmEngine,
"pkcs5",
)
),
MutableDict.as_mutable(encrypted_type(type_in=JSONTypeOverride)),
nullable=True,
) # Type bytea in the db
single_row = Column(
Expand Down
13 changes: 2 additions & 11 deletions src/fides/api/models/masking_secret.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,10 @@
from sqlalchemy import Column, ForeignKey, String
from sqlalchemy.ext.declarative import declared_attr
from sqlalchemy.orm import Session, relationship
from sqlalchemy_utils.types.encrypted.encrypted_type import (
AesGcmEngine,
StringEncryptedType,
)

from fides.api.db.base_class import Base, JSONTypeOverride
from fides.api.db.encryption_utils import encrypted_type
from fides.api.util.custom_json_encoder import ENCODED_BYTES_PREFIX
from fides.config import CONFIG

if TYPE_CHECKING:
from fides.api.models.privacy_request import PrivacyRequest
Expand All @@ -30,12 +26,7 @@ def __tablename__(self) -> str:
String, ForeignKey("privacyrequest.id", ondelete="CASCADE"), nullable=False
)
secret = Column(
StringEncryptedType(
JSONTypeOverride,
CONFIG.security.app_encryption_key,
AesGcmEngine,
"pkcs5",
),
encrypted_type(type_in=JSONTypeOverride),
nullable=False,
)
masking_strategy = Column(String, nullable=False)
Expand Down
15 changes: 2 additions & 13 deletions src/fides/api/models/messaging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,10 @@
from sqlalchemy.dialects.postgresql import JSONB
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy.orm import Session
from sqlalchemy_utils.types.encrypted.encrypted_type import (
AesGcmEngine,
StringEncryptedType,
)

from fides.api.common_exceptions import MessageDispatchException
from fides.api.db.base_class import Base, JSONTypeOverride
from fides.api.db.encryption_utils import encrypted_type
from fides.api.schemas.messaging.messaging import (
EMAIL_MESSAGING_SERVICES,
SMS_MESSAGING_SERVICES,
Expand All @@ -33,7 +30,6 @@
PossibleMessagingSecrets,
)
from fides.api.util.logger import Pii
from fides.config import CONFIG
from fides.config.config_proxy import ConfigProxy


Expand Down Expand Up @@ -92,14 +88,7 @@ class MessagingConfig(Base):
last_test_timestamp = Column(DateTime(timezone=True))
last_test_succeeded = Column(Boolean)
secrets = Column(
MutableDict.as_mutable(
StringEncryptedType(
JSONTypeOverride,
CONFIG.security.app_encryption_key,
AesGcmEngine,
"pkcs5",
)
),
MutableDict.as_mutable(encrypted_type(type_in=JSONTypeOverride)),
nullable=True,
) # Type bytea in the db

Expand Down
Loading
Loading