Skip to content

Commit de7e37b

Browse files
committed
feat: add a Fernet-backed at-rest encryption helper keyed by ENCRYPTION_KEY
Introduce app/core/encryption.py with encrypt / decrypt / is_encrypted and a DecryptionError, plus a required ENCRYPTION_KEY setting on the core Settings class with startup validation. No storage consumer is wired here. The key is required in every environment and no value ships in the repository. A committed development key was considered and rejected: the repository is public, FASTAPI_ENV defaults to development so an unconfigured source install runs on that profile, and the values this protects are live third-party credentials. The test suite mints a key per run at tests/conftest.py import, the migration check in CI generates one per job, and a developer supplies their own. A test asserts no profile in settings.yaml carries the key. ENCRYPTION_KEY is declared SecretStr | None so the model validator can produce an actionable message for the absent case: pydantic resolves required-field presence before any after validator, so a required SecretStr would fail with a generic "Field required" instead. None is a sentinel for absent, never a usable key, and nothing derives the key from SECRET_KEY. is_encrypted reads Fernet's own version marker rather than attempting a decrypt, so a token written under a different key still reports True. A migration must branch on it rather than on a caught DecryptionError, which cannot separate legacy plaintext from ciphertext this process cannot read.
1 parent a157c14 commit de7e37b

13 files changed

Lines changed: 599 additions & 3 deletions

File tree

.github/instructions/security.instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Scope is the *surface being written*, not its language: a diff that is entirely
1010

1111
## Must flag (Critical)
1212

13-
- **Hardcoded secrets** — API keys, OAuth client secrets, DB credentials, JWT signing keys, passwords. Secrets come from environment variables (optionally via a local `.env`) or a **mounted secret file** — one file per canonical `__`-nested variable name (`SECRET_KEY`, `SEP__DATABASE__PASSWORD`) in the directory `SECRETS_DIR` names, which keeps the value out of the process environment. Never in `settings.yaml`, source code, fixtures, or config. `.env` is gitignored; a `.env` entry outranks a secret file, and env vars outrank all other config sources.
13+
- **Hardcoded secrets** — API keys, OAuth client secrets, DB credentials, JWT signing keys, passwords. Secrets come from environment variables (optionally via a local `.env`) or a **mounted secret file** — one file per canonical `__`-nested variable name (`SECRET_KEY`, `SEP__DATABASE__PASSWORD`) in the directory `SECRETS_DIR` names, which keeps the value out of the process environment. Never in `settings.yaml`, source code, fixtures, or config. `.env` is gitignored; a `.env` entry outranks a secret file, and env vars outrank all other config sources. **This holds even for a value that only development would use**, and even when making it mandatory breaks every existing checkout until each developer supplies one: the repository is public, so a committed key is world-readable and stays in history after any later removal, and `FASTAPI_ENV` defaults to `development`, so a "development-only" credential is what an unconfigured source install actually runs on. `ENCRYPTION_KEY` is the worked example — it has no committed value in any profile, and a test asserts that none is ever added.
1414
- **A secret expanded into a command line reaches the failure path that prints it.** `curl -u "$USER:$API_TOKEN"` and relatives — any credential expanded inline into a command a shell, `Makefile` recipe, or Python subprocess builds — behave perfectly until the command fails and something on the failure path prints the command it ran (a `CalledProcessError` carrying `cmd`, a shell trace, a wrapper that echoes before raising). Prefer a channel that keeps the secret off the command line: `curl --netrc-file` / `--config` over `-u`, the tool's env-var or credentials-file input over a token in `argv`. Where no such channel exists, catch the failure yourself rather than letting a default handler render `cmd`. A credential that reaches a log, transcript, or CI record is an **incident, not a note** — the only remedy that restores the prior state is rotation.
1515
- **Raw SQL with user-controlled input**`session.execute(text(...))` or f-string SQL where any part comes from a request parameter, form field, URL path, header, or JSON body. All DB access goes through CRUD managers, which parameterise.
1616
- **`| safe` / `Markup()` on user-controlled data** defeats auto-escaping. The server-rendered UI is gone — the Jinja SSR layer was deleted — so the only Jinja surface left is the **report PDF renderer**`app/sep/apps/report/templates/result_pdf.html.j2` and the filters in `app/sep/utils/jinja.py`, which returns `Markup` from its syntax highlighter. Task output and snippet arguments reach that template, so the risk is live even though nothing is served to a browser.

.github/workflows/python.yaml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,11 @@ jobs:
5959
uses: ./.github/actions/setup-python-job
6060
with:
6161
python-version: ${{ matrix.python }}
62+
# ENCRYPTION_KEY is required to construct settings and is minted per run
63+
# rather than stored: migrations encrypt nothing, so the value only has to
64+
# be a valid key, and nothing here needs it to outlive the job.
6265
- name: Check migrations
63-
run: AUTH__PROVIDER__CASDOOR__CLIENT_SECRET=notatoken AUTH__PROVIDER__CASDOOR__CLIENT_ID=notatoken make checkmigrations
66+
run: AUTH__PROVIDER__CASDOOR__CLIENT_SECRET=notatoken AUTH__PROVIDER__CASDOOR__CLIENT_ID=notatoken ENCRYPTION_KEY="$(make -s encryption-key)" make checkmigrations
6467

6568
test:
6669
name: test (${{ matrix.python }})

Makefile

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,12 @@ check-nomad-payload-size: venv
222222
check-sidecar-purge: venv
223223
@$(DARWIN_DYLD) "${VENV_BIN}"/python scripts/check_sidecar_purge.py $(ARGS)
224224

225+
# A Fernet key is 32 random bytes in url-safe base64, so this needs neither the
226+
# venv nor cryptography: an operator runs it on a fresh checkout to copy the one
227+
# line it prints, and the venv bootstrap would both fail there and bury the key.
228+
encryption-key:
229+
@$(PYTHON) -c 'import base64, os; print(base64.urlsafe_b64encode(os.urandom(32)).decode())'
230+
225231
changelog-add:
226232
ifndef TICKET
227233
$(error TICKET is required. Usage: make changelog-add TICKET=SEP-XXX SECTION=added MSG="description")
@@ -371,4 +377,4 @@ lint-pipelines:
371377
done; \
372378
if [ "$${failures}" -ne 0 ]; then exit 1; fi
373379

374-
.PHONY: venv build pack builder image format ruff typecheck lint audit run-pre-commit dev-backend dev-frontend backfill-legacy-forms pip-audit bandit makemigrations makemigrations-plugin migrate checkmigrations mergemigrations test regen-specs regen-pbm-payloads regen-pbm-payloads-check regen-xtrabackup-variants regen-xtrabackup-variants-check smoke-xtrabackup-variants check-nomad-payload-size check-sidecar-purge release-prep release-rc release-stable trigger-jenkins lint-pipelines changelog-add changelog-check changelog-list startapp startapp-check
380+
.PHONY: venv build pack builder image format ruff typecheck lint audit run-pre-commit dev-backend dev-frontend backfill-legacy-forms pip-audit bandit makemigrations makemigrations-plugin migrate checkmigrations mergemigrations test regen-specs regen-pbm-payloads regen-pbm-payloads-check regen-xtrabackup-variants regen-xtrabackup-variants-check smoke-xtrabackup-variants check-nomad-payload-size check-sidecar-purge release-prep release-rc release-stable trigger-jenkins lint-pipelines encryption-key changelog-add changelog-check changelog-list startapp startapp-check

README.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,34 @@ You can create a basic .env file template by running the following command in th
334334
echo -e "AUTH__PROVIDER__CASDOOR__CLIENT_ID=YOUR_CASDOOR_CLIENT_ID\nAUTH__PROVIDER__CASDOOR__CLIENT_SECRET=YOUR_CASDOOR_CLIENT_SECRET\n" > .env
335335
```
336336

337+
#### `ENCRYPTION_KEY`
338+
339+
SEP encrypts some of the values it stores in its own databases. **Every
340+
environment needs its own `ENCRYPTION_KEY`, local development included** — SEP
341+
refuses to start without one, and so do the Celery workers, the Alembic
342+
migrations, and the OpenAPI dump. It has no default, is never derived from
343+
`SECRET_KEY`, and no value ships in the repository: the values it protects are
344+
real third-party credentials, so a shared key would protect nothing from anyone
345+
who can read the source.
346+
347+
Mint one and add it to your `.env`:
348+
349+
```shell
350+
echo "ENCRYPTION_KEY=$(make -s encryption-key)" >> .env
351+
```
352+
353+
`openssl rand -base64 32` works too. Note that `openssl rand -hex 32` — the
354+
generator `SECRET_KEY` uses — does **not** produce a valid key.
355+
356+
A deployment supplies the same value as an environment variable or as a file
357+
named `ENCRYPTION_KEY` under `SECRETS_DIR`.
358+
359+
**Keep the value stable.** Ciphertext outlives the process that wrote it, so
360+
rotating or losing the key makes every already-encrypted row permanently
361+
unreadable. There is no recovery path and no rotation tooling.
362+
363+
The test suite needs no action — it mints its own key per run.
364+
337365
#### Supplying a setting as a mounted file
338366

339367
Any setting can instead be supplied as a file inside the directory `SECRETS_DIR` names,

app/core/config.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
)
4040
from urllib.parse import urlparse
4141

42+
from cryptography.fernet import Fernet
4243
from fastapi import APIRouter, FastAPI, params
4344
from fastapi.applications import AppType
4445
from fastapi.middleware.cors import CORSMiddleware
@@ -431,6 +432,19 @@ def hostname(self) -> str | None:
431432

432433
_INTERNAL_TOKEN_LABEL = b"sep-internal-token"
433434

435+
_ENCRYPTION_KEY_ERROR = (
436+
"ENCRYPTION_KEY must be set to a valid Fernet key (32 url-safe "
437+
"base64-encoded bytes). Generate one with `make encryption-key` or `openssl "
438+
"rand -base64 32`, then add it to your .env as ENCRYPTION_KEY=<key>, export "
439+
"it, or mount it as a file named ENCRYPTION_KEY under SECRETS_DIR. It has no "
440+
"default and is never derived from SECRET_KEY."
441+
)
442+
"""The remediation for an unset, empty, or malformed ``ENCRYPTION_KEY``.
443+
444+
``openssl rand -hex 32``, which ``SECRET_KEY``'s own message offers, produces 64
445+
characters Fernet rejects, so the two remediations are deliberately different.
446+
"""
447+
434448
SettingsOverrideKey = Annotated[str, StringConstraints(pattern=r"^[^\s.]+\.[^\s.]+$")]
435449

436450

@@ -628,6 +642,14 @@ class Settings(BaseYamlSettings):
628642
every process sharing ``SECRET_KEY`` resolves the identical token.
629643
Generate an explicit value with ``openssl rand -hex 32`` to rotate it
630644
independently of ``SECRET_KEY``.
645+
:param ENCRYPTION_KEY: The Fernet key :mod:`app.core.encryption` uses to
646+
encrypt values SEP stores in its own databases. It has no default and is
647+
never derived from ``SECRET_KEY``: ciphertext outlives the process that
648+
wrote it, so a key that changed on restart would orphan every encrypted
649+
row. Every environment supplies its own, as an environment variable or
650+
as a file named ``ENCRYPTION_KEY`` under ``SECRETS_DIR``. Nothing is
651+
committed: a key in the repository would be readable by anyone who can
652+
read the repository, and the values it protects are real credentials.
631653
:param LOGGING: The logging level for the application. Defaults to LogLevel.WARNING.
632654
:param LOGGING_CONFIG: dictConfig logging configuration.
633655
:param SSL_CAFILE: The SSL CA file to use for remote API requests.
@@ -650,6 +672,7 @@ class Settings(BaseYamlSettings):
650672
ALLOW_CONCURRENT_SESSIONS: bool = False
651673
SECRET_KEY: SecretStr = SecretStr(secrets.token_urlsafe(32))
652674
SEP_INTERNAL_TOKEN: SecretStr | None = None
675+
ENCRYPTION_KEY: SecretStr | None = None
653676
LOGGING: LogLevel = hot_field(LogLevel.WARNING) # ty: ignore[invalid-assignment]
654677
LOGGING_CONFIG: dict[str, Any] = {}
655678
SSL_CAFILE: RelativeFilePathField | None = None
@@ -732,6 +755,29 @@ def derive_internal_token(self) -> Self:
732755
self.SEP_INTERNAL_TOKEN = SecretStr(derived)
733756
return self
734757

758+
@model_validator(mode="after")
759+
def validate_encryption_key(self) -> Self:
760+
"""Reject an unset, empty, or malformed ``ENCRYPTION_KEY``.
761+
762+
The field is declared ``SecretStr | None`` rather than required so this
763+
validator runs at all: pydantic resolves required-field presence before
764+
any ``after`` model validator, so an absent required key would fail with
765+
a generic ``Field required`` instead of the remediation below. ``None``
766+
is a sentinel for "absent", never a usable key: every reachable
767+
``encrypt`` / ``decrypt`` call has passed this check.
768+
769+
:return: Validated settings carrying a usable ``ENCRYPTION_KEY``.
770+
:raises ValueError: If the key is unset, empty, or not a valid Fernet key.
771+
"""
772+
key = self.ENCRYPTION_KEY.get_secret_value() if self.ENCRYPTION_KEY else ""
773+
if not key:
774+
raise ValueError(_ENCRYPTION_KEY_ERROR)
775+
try:
776+
Fernet(key.encode())
777+
except ValueError as exc:
778+
raise ValueError(_ENCRYPTION_KEY_ERROR) from exc
779+
return self
780+
735781
@classmethod
736782
def settings_customise_sources(
737783
cls,

app/core/encryption.py

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
# Copyright (C) 2026 Percona LLC
2+
#
3+
# This program is free software: you can redistribute it and/or modify
4+
# it under the terms of the GNU Affero General Public License as published by
5+
# the Free Software Foundation, either version 3 of the License, or
6+
# (at your option) any later version.
7+
#
8+
# This program is distributed in the hope that it will be useful,
9+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
10+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11+
# GNU Affero General Public License for more details.
12+
#
13+
# You should have received a copy of the GNU Affero General Public License
14+
# along with this program. If not, see <https://www.gnu.org/licenses/>.
15+
16+
"""Encrypt and decrypt values SEP stores at rest, keyed by ``ENCRYPTION_KEY``.
17+
18+
The ciphertext is Fernet: authenticated AES-128-CBC carrying its own version
19+
marker, timestamp and HMAC, rendered as URL-safe base64 text that any ``str``
20+
or JSON column stores unchanged. Encryption is **not** deterministic: each call
21+
derives a fresh IV, so two encryptions of one plaintext differ and ciphertext
22+
can never be compared for equality.
23+
24+
Use :func:`is_encrypted`, never a caught :class:`DecryptionError`, to decide
25+
whether a stored value still needs encrypting.
26+
"""
27+
28+
__all__ = ["DecryptionError", "decrypt", "encrypt", "is_encrypted"]
29+
30+
import base64
31+
from functools import lru_cache
32+
33+
from cryptography.fernet import Fernet, InvalidToken
34+
35+
from app.core.config import settings
36+
37+
_FERNET_VERSION = 0x80
38+
"""The first byte of every decoded Fernet token, which is its version marker."""
39+
40+
_MIN_TOKEN_BYTES = 73
41+
"""The shortest decodable Fernet token: version, timestamp, IV, one block, HMAC.
42+
43+
CBC pads even an empty plaintext to a full 16-byte block, so no shorter value is
44+
decryptable. Accepting one would make a migration *skip* a value it can never
45+
decrypt, leaving it in the clear for good.
46+
"""
47+
48+
49+
class DecryptionError(ValueError):
50+
"""Define exception raised when a value cannot be decrypted with the configured key."""
51+
52+
53+
@lru_cache(maxsize=1)
54+
def _get_fernet() -> Fernet:
55+
"""Return the process-wide cipher built from ``settings.ENCRYPTION_KEY``.
56+
57+
Cached so the key is resolved once per process; ``cache_clear()`` resets it
58+
between tests. Deferred behind an accessor rather than built at module
59+
scope so importing this module resolves no settings.
60+
61+
:return: The cached cipher.
62+
:raises RuntimeError: If ``ENCRYPTION_KEY`` is unset.
63+
:raises ValueError: Propagates from ``Fernet`` if ``ENCRYPTION_KEY`` is set
64+
but malformed.
65+
:meth:`~app.core.config.Settings.validate_encryption_key` refuses to
66+
construct settings in either case, so both mean a patched environment.
67+
"""
68+
key = settings.ENCRYPTION_KEY
69+
if key is None:
70+
raise RuntimeError("ENCRYPTION_KEY must be configured.")
71+
return Fernet(key.get_secret_value().encode())
72+
73+
74+
def encrypt(value: str) -> str:
75+
"""Return ``value`` encrypted as URL-safe base64 ciphertext text.
76+
77+
:param value: The plaintext to encrypt.
78+
:return: The ciphertext, storable in any text or JSON column.
79+
:raises RuntimeError: Propagates from :func:`_get_fernet` when
80+
``ENCRYPTION_KEY`` is unset.
81+
"""
82+
return _get_fernet().encrypt(value.encode()).decode("ascii")
83+
84+
85+
def decrypt(value: str) -> str:
86+
"""Return the plaintext behind ``value``.
87+
88+
``value`` is encoded here rather than handed over as text: Fernet narrows a
89+
``str`` token with ``ascii``, and :mod:`base64` turns that failure into a
90+
plain ``ValueError`` its ``binascii.Error`` handler does not catch, so a
91+
non-ASCII stored value would escape uncaught rather than as the failure this
92+
function documents.
93+
94+
:param value: The ciphertext to decrypt.
95+
:return: The decrypted plaintext.
96+
:raises DecryptionError: If ``value`` is not ciphertext this key produced,
97+
which covers a legacy plaintext value, a corrupt one, and one encrypted
98+
under a different key alike. Use :func:`is_encrypted` to tell those
99+
apart; this exception does not.
100+
:raises RuntimeError: Propagates from :func:`_get_fernet` when
101+
``ENCRYPTION_KEY`` is unset.
102+
"""
103+
try:
104+
return _get_fernet().decrypt(value.encode()).decode()
105+
except InvalidToken as exc:
106+
raise DecryptionError(
107+
"Value could not be decrypted: it is malformed, or was encrypted "
108+
"with a different ENCRYPTION_KEY."
109+
) from exc
110+
111+
112+
def is_encrypted(value: str) -> bool:
113+
"""Return whether ``value`` is structurally a Fernet token.
114+
115+
Reads the token's own version marker instead of attempting a decrypt, so a
116+
token written under a *different* key still reports ``True``. That is the
117+
property a migration needs: a caught :class:`DecryptionError` cannot
118+
separate "never encrypted" from "encrypted with a key this process does not
119+
hold", and encrypting the latter again destroys the only copy of its
120+
plaintext.
121+
122+
:param value: The stored value to classify.
123+
:return: ``True`` when ``value`` is shaped like a Fernet token, ``False``
124+
for anything else, including input that is not valid base64 at all.
125+
"""
126+
try:
127+
raw = base64.urlsafe_b64decode(value)
128+
except ValueError:
129+
return False
130+
return len(raw) >= _MIN_TOKEN_BYTES and raw[0] == _FERNET_VERSION

changelog.d/SEP-1972.breaking.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ENCRYPTION_KEY is now required in every environment, local development included: SEP, its Celery workers and its Alembic migrations all refuse to start until it is supplied. Existing installations must add one before upgrading. See the Configuration Changes entry for how to generate and supply it.

changelog.d/SEP-1972.config.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
SEP now requires an ENCRYPTION_KEY setting in every environment, supplied as an environment variable or as a file named ENCRYPTION_KEY under SECRETS_DIR; it has no default, is never derived from SECRET_KEY, and no value ships in the repository. Generate one with "make encryption-key" or "openssl rand -base64 32" ("openssl rand -hex 32" does not produce a valid key), and keep the value stable, since rotating it makes already-encrypted data unreadable. Nothing is encrypted yet; the key gates the encryption primitive itself.

0 commit comments

Comments
 (0)