diff --git a/backend/app/services/credential_crypto.py b/backend/app/services/credential_crypto.py index 86a45ce..273e574 100644 --- a/backend/app/services/credential_crypto.py +++ b/backend/app/services/credential_crypto.py @@ -1,40 +1,82 @@ """Fernet-based encryption for credential blobs. The key is a single 32-byte value (urlsafe-base64-encoded; what -Fernet.generate_key produces) stored at a fixed path inside the -images/data root. Created on first boot if absent; mode 0600. No KDF -needed — the file contents are already maximum-entropy random bytes. +Fernet.generate_key produces) stored at /images/secrets/credential_key.b64 +(mode 0600, parent dir 0700). The 2026-06-02 audit caught a silent +key-regeneration path: on a partial disaster restore where the DB was +restored but the secrets dir was lost, the old `_load_or_create_key` +would mint a fresh key with no log, producing a working-looking system +where every authenticated download failed AUTH_ERROR until the operator +re-uploaded every credential by hand. Now the constructor refuses to +auto-generate unless either: -Operator backup procedure must include this file alongside the rest -of /images/ — losing it makes existing encrypted_blob rows -undecryptable (recovery = delete the rows and re-upload). + * the caller explicitly passes `bootstrap_ok=True` (tests, scripts), or + * the env var `CURATOR_BOOTSTRAP_NEW_KEY=1` is set (operator opt-in + during first-time setup). + +Otherwise it raises `MissingCredentialKey` so the app fails fast at +startup and the operator can restore the key file from backup. + +Operator backup procedure must include /images/secrets/ alongside the +rest of /images/ — losing the key file makes existing encrypted_blob +rows undecryptable (recovery = delete the rows and re-upload). """ +import logging import os from pathlib import Path from cryptography.fernet import Fernet, InvalidToken +log = logging.getLogger(__name__) + +_BOOTSTRAP_ENV_VAR = "CURATOR_BOOTSTRAP_NEW_KEY" + class InvalidCredentialBlob(Exception): """Raised when decryption fails (wrong key, tampered blob, …).""" +class MissingCredentialKey(Exception): + """The Fernet key file is missing AND the caller hasn't opted in to + generating a new one. Audit 2026-06-02: prevents silent key + regeneration on partial DB-restored / secrets-lost deployments. + Set CURATOR_BOOTSTRAP_NEW_KEY=1 for first-time setup, or restore the + key file from backup.""" + + class CredentialCrypto: """Fernet encrypt/decrypt with an on-disk key file. - Instantiate with a path; the file is created on first access and - reused thereafter. Tests pass a tmp_path; production calls with + Instantiate with a path; the file is loaded if present, or created + if absent AND the caller has opted in (bootstrap_ok=True or + CURATOR_BOOTSTRAP_NEW_KEY=1 env var). Production sites: `IMAGES_ROOT / "secrets" / "credential_key.b64"`. """ - def __init__(self, key_path: Path): + def __init__(self, key_path: Path, *, bootstrap_ok: bool | None = None): self._key_path = Path(key_path) - self._fernet = Fernet(self._load_or_create_key()) + if bootstrap_ok is None: + bootstrap_ok = os.environ.get(_BOOTSTRAP_ENV_VAR) == "1" + self._fernet = Fernet(self._load_or_create_key(bootstrap_ok)) - def _load_or_create_key(self) -> bytes: + def _load_or_create_key(self, bootstrap_ok: bool) -> bytes: if self._key_path.exists(): return self._key_path.read_bytes() + if not bootstrap_ok: + raise MissingCredentialKey( + f"Fernet key file not found at {self._key_path}. " + f"For first-time setup, set {_BOOTSTRAP_ENV_VAR}=1. " + f"If this is a restored instance, restore the key file " + f"from backup — generating a new one would make every " + f"existing Credential row undecryptable." + ) + log.warning( + "Generating NEW Fernet credential key at %s. Any existing " + "encrypted_blob rows in the DB will be undecryptable — " + "re-upload each credential after this completes.", + self._key_path, + ) parent = self._key_path.parent parent.mkdir(parents=True, exist_ok=True) os.chmod(parent, 0o700) diff --git a/tests/test_credential_crypto.py b/tests/test_credential_crypto.py index 199b838..6274bb5 100644 --- a/tests/test_credential_crypto.py +++ b/tests/test_credential_crypto.py @@ -14,7 +14,7 @@ pytestmark = pytest.mark.integration def test_load_or_create_writes_key_with_mode_0600(tmp_path): key_path = tmp_path / "secrets" / "credential_key.b64" - CredentialCrypto(key_path) + CredentialCrypto(key_path, bootstrap_ok=True) assert key_path.exists() mode = stat.S_IMODE(os.stat(key_path).st_mode) assert mode == 0o600 @@ -25,9 +25,9 @@ def test_load_or_create_writes_key_with_mode_0600(tmp_path): def test_load_existing_key_is_idempotent(tmp_path): key_path = tmp_path / "credential_key.b64" - crypto1 = CredentialCrypto(key_path) + crypto1 = CredentialCrypto(key_path, bootstrap_ok=True) contents_after_first = key_path.read_bytes() - crypto2 = CredentialCrypto(key_path) + crypto2 = CredentialCrypto(key_path, bootstrap_ok=True) contents_after_second = key_path.read_bytes() assert contents_after_first == contents_after_second # And both crypto instances decrypt each other's ciphertext @@ -36,7 +36,7 @@ def test_load_existing_key_is_idempotent(tmp_path): def test_encrypt_decrypt_round_trip(tmp_path): - crypto = CredentialCrypto(tmp_path / "k") + crypto = CredentialCrypto(tmp_path / "k", bootstrap_ok=True) plaintext = "domain.com\tTRUE\t/\tTRUE\t1700000000\tname\tvalue" ct = crypto.encrypt(plaintext) assert isinstance(ct, bytes) @@ -45,8 +45,27 @@ def test_encrypt_decrypt_round_trip(tmp_path): def test_decrypt_with_wrong_key_raises(tmp_path): - crypto_a = CredentialCrypto(tmp_path / "a") - crypto_b = CredentialCrypto(tmp_path / "b") + crypto_a = CredentialCrypto(tmp_path / "a", bootstrap_ok=True) + crypto_b = CredentialCrypto(tmp_path / "b", bootstrap_ok=True) ct = crypto_a.encrypt("secret") with pytest.raises(InvalidCredentialBlob): crypto_b.decrypt(ct) + + +def test_missing_key_without_bootstrap_raises(tmp_path, monkeypatch): + """Audit 2026-06-02: without explicit opt-in, a missing key file + is a fatal startup error — silent regeneration on partial restore + would make every existing Credential row undecryptable.""" + from backend.app.services.credential_crypto import MissingCredentialKey + monkeypatch.delenv("CURATOR_BOOTSTRAP_NEW_KEY", raising=False) + with pytest.raises(MissingCredentialKey): + CredentialCrypto(tmp_path / "absent.b64") + + +def test_missing_key_with_env_var_bootstraps(tmp_path, monkeypatch): + """The env var CURATOR_BOOTSTRAP_NEW_KEY=1 is the operator's + first-time-setup opt-in for auto-creating the key file.""" + monkeypatch.setenv("CURATOR_BOOTSTRAP_NEW_KEY", "1") + key_path = tmp_path / "bootstrap.b64" + CredentialCrypto(key_path) # no bootstrap_ok kwarg — relies on env + assert key_path.exists() diff --git a/tests/test_credential_service.py b/tests/test_credential_service.py index b856761..b9857ec 100644 --- a/tests/test_credential_service.py +++ b/tests/test_credential_service.py @@ -21,7 +21,7 @@ _NETSCAPE = ( @pytest.fixture def crypto(tmp_path): - return CredentialCrypto(tmp_path / "k") + return CredentialCrypto(tmp_path / "k", bootstrap_ok=True) @pytest.mark.asyncio diff --git a/tests/test_download_service.py b/tests/test_download_service.py index 65ea850..cc07f05 100644 --- a/tests/test_download_service.py +++ b/tests/test_download_service.py @@ -130,7 +130,7 @@ async def test_download_source_attaches_written_files( thumbnailer=Thumbnailer(images_root=images_root), settings=sync_settings, ) - crypto = CredentialCrypto(tmp_path / "key.b64") + crypto = CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True) cred_service = CredentialService(db, crypto) svc = DownloadService( @@ -405,7 +405,7 @@ async def test_backfill_decrements_after_run( session=db_sync, images_root=images_root, import_root=images_root, thumbnailer=Thumbnailer(images_root=images_root), settings=sync_settings, ) - cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64")) + cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True)) svc = DownloadService( async_session=db, sync_session=db_sync, gdl=fake_gdl, importer=importer, cred_service=cred_service, @@ -446,7 +446,7 @@ async def test_backfill_auto_resets_on_clean_zero_files( thumbnailer=Thumbnailer(images_root=tmp_path / "images"), settings=sync_settings, ) - cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64")) + cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True)) svc = DownloadService( async_session=db, sync_session=db_sync, gdl=fake_gdl, importer=importer, cred_service=cred_service, @@ -484,7 +484,7 @@ async def test_tick_mode_does_not_touch_backfill_counter( thumbnailer=Thumbnailer(images_root=tmp_path / "images"), settings=sync_settings, ) - cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64")) + cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True)) svc = DownloadService( async_session=db, sync_session=db_sync, gdl=fake_gdl, importer=importer, cred_service=cred_service, @@ -529,7 +529,7 @@ async def test_partial_error_type_maps_to_ok_status( session=db_sync, images_root=images_root, import_root=images_root, thumbnailer=Thumbnailer(images_root=images_root), settings=sync_settings, ) - cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64")) + cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True)) svc = DownloadService( async_session=db, sync_session=db_sync, gdl=fake_gdl, importer=importer, cred_service=cred_service, @@ -582,7 +582,7 @@ async def test_download_enqueues_thumbnail_and_ml_per_attached_image( session=db_sync, images_root=images_root, import_root=images_root, thumbnailer=Thumbnailer(images_root=images_root), settings=sync_settings, ) - cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64")) + cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True)) # Capture the IDs that the orchestrator hands off to each Celery task. # The .delay() shim runs inside DownloadService._phase3_persist (lazy