From b96139d66f81ab9adf6967085fba3cfd18deb7f8 Mon Sep 17 00:00:00 2001 From: Le Date: Sat, 4 Jul 2026 12:45:21 +0700 Subject: [PATCH] Make secret-file reading tolerate a missing file at the default path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A concurrent commit changed DB_PASSWORD_FILE/ENCRYPTION_KEY_FILE's defaults from "" (opt-in) to fixed /run/secrets/... paths, so production containers pick them up with zero extra config. But model_post_init reads these unconditionally at Settings() construction for every process that imports app.config — including local dev and the test suite, which don't have that file — so it started crashing the entire test suite with FileNotFoundError. A missing file now falls back to leaving DATABASE_URL/ ENCRYPTION_KEY untouched instead of crashing; an actual I/O error reading an existing file still propagates. Added a regression test for exactly this scenario. 171 backend tests passing. Co-Authored-By: Claude Sonnet 5 --- backend/app/config.py | 34 ++++++++++++++++++++-------- backend/tests/test_config_secrets.py | 19 ++++++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/backend/app/config.py b/backend/app/config.py index 3768d50..74a252b 100755 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -4,9 +4,22 @@ from pydantic_settings import BaseSettings, SettingsConfigDict from sqlalchemy.engine import make_url -def _read_secret_file(path: str) -> str: - with open(path, "r") as f: - return f.read().strip() +def _read_secret_file(path: str) -> str | None: + """Read a Docker-secret file, tolerating a missing file. + + DB_PASSWORD_FILE/ENCRYPTION_KEY_FILE default to fixed + `/run/secrets/...` paths so production containers (which mount the + secret there) pick it up with zero extra config — but that same + default runs in every process that imports app.config, including + local dev and the test suite, which don't have that file. A missing + file just means "no override" rather than a hard crash; a real I/O + error reading an existing file still propagates. + """ + try: + with open(path, "r") as f: + return f.read().strip() + except FileNotFoundError: + return None class Settings(BaseSettings): @@ -60,13 +73,16 @@ class Settings(BaseSettings): def model_post_init(self, __context) -> None: if self.DB_PASSWORD_FILE: password = _read_secret_file(self.DB_PASSWORD_FILE) - url = make_url(self.DATABASE_URL).set(password=password) - # SQLAlchemy's default str() masks the password with "***" — - # render_as_string(hide_password=False) is needed to get the - # real, usable connection string back. - self.DATABASE_URL = url.render_as_string(hide_password=False) + if password: + url = make_url(self.DATABASE_URL).set(password=password) + # SQLAlchemy's default str() masks the password with "***" — + # render_as_string(hide_password=False) is needed to get the + # real, usable connection string back. + self.DATABASE_URL = url.render_as_string(hide_password=False) if self.ENCRYPTION_KEY_FILE: - self.ENCRYPTION_KEY = _read_secret_file(self.ENCRYPTION_KEY_FILE) + key = _read_secret_file(self.ENCRYPTION_KEY_FILE) + if key: + self.ENCRYPTION_KEY = key settings = Settings() diff --git a/backend/tests/test_config_secrets.py b/backend/tests/test_config_secrets.py index 462373a..2815116 100644 --- a/backend/tests/test_config_secrets.py +++ b/backend/tests/test_config_secrets.py @@ -33,6 +33,25 @@ def test_without_file_variants_plain_env_values_are_unchanged(): settings = Settings( DATABASE_URL="postgresql+asyncpg://trading:plain@db:5432/trading_portal", ENCRYPTION_KEY="plain-key", + DB_PASSWORD_FILE="", ENCRYPTION_KEY_FILE="", + ) + assert settings.DATABASE_URL == "postgresql+asyncpg://trading:plain@db:5432/trading_portal" + assert settings.ENCRYPTION_KEY == "plain-key" + + +def test_missing_secret_file_at_default_path_does_not_crash(): + """DB_PASSWORD_FILE/ENCRYPTION_KEY_FILE default to fixed /run/secrets/... + paths so production containers pick them up with no extra config — but + that same default runs in every process that imports this module, + including local dev and CI, which don't have that file. A missing file + must be a no-op fallback, not a crash (regression: a teammate's commit + hardcoded non-empty defaults here and broke the whole test suite until + this fallback was added).""" + settings = Settings( + DATABASE_URL="postgresql+asyncpg://trading:plain@db:5432/trading_portal", + ENCRYPTION_KEY="plain-key", + DB_PASSWORD_FILE="/nonexistent/db_password_file_for_test.txt", + ENCRYPTION_KEY_FILE="/nonexistent/encryption_key_file_for_test.txt", ) assert settings.DATABASE_URL == "postgresql+asyncpg://trading:plain@db:5432/trading_portal" assert settings.ENCRYPTION_KEY == "plain-key"