Skip to content

Commit 054fb31

Browse files
committed
feat: add SECRETS_DIR existence and writability warnings
Add check_secrets_dir() helper that validates the configured SECRETS_DIR path and returns warning messages for non-existent or writable directories. Called from the lifespan after logging is configured, using the same dual-logger pattern (uvicorn + loguru) as CORS and Redis warnings. Skip passing _secrets_dir to pydantic-settings when the path doesn't exist to prevent its raw warnings.warn() during early startup. Signed-off-by: Grant Ramsay <seapagan@gmail.com>
1 parent ed42856 commit 054fb31

3 files changed

Lines changed: 72 additions & 5 deletions

File tree

app/config/settings.py

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import os
66
import sys
77
from functools import lru_cache
8-
from pathlib import Path # noqa: TC003
8+
from pathlib import Path
99
from typing import TYPE_CHECKING, cast
1010
from urllib.parse import quote
1111

@@ -300,11 +300,39 @@ def validate_db_user(cls: type[Settings], value: str) -> str:
300300
return value
301301

302302

303+
def check_secrets_dir() -> list[str]:
304+
"""Validate SECRETS_DIR and return warning messages.
305+
306+
Returns a list of warning strings (empty if no issues found).
307+
Intended to be called from the application lifespan, after
308+
logging is configured.
309+
"""
310+
warnings: list[str] = []
311+
secrets_dir = os.getenv("SECRETS_DIR", "").strip()
312+
if not secrets_dir:
313+
return warnings
314+
315+
secrets_path = Path(secrets_dir)
316+
if not secrets_path.is_dir():
317+
warnings.append(
318+
f"SECRETS_DIR '{secrets_dir}' does not exist or is "
319+
f"not a directory. File-based secrets will not be "
320+
f"loaded."
321+
)
322+
elif os.access(secrets_path, os.W_OK):
323+
warnings.append(
324+
f"SECRETS_DIR '{secrets_dir}' is writable by the "
325+
f"current process. For best security, this "
326+
f"directory should be read-only."
327+
)
328+
return warnings
329+
330+
303331
@lru_cache
304332
def get_settings() -> Settings:
305333
"""Return the current settings."""
306-
secrets_dir = os.getenv("SECRETS_DIR")
334+
secrets_dir = os.getenv("SECRETS_DIR", "").strip()
307335
settings_factory = cast("Callable[..., Settings]", Settings)
308-
if secrets_dir and secrets_dir.strip():
336+
if secrets_dir and Path(secrets_dir).is_dir():
309337
return settings_factory(_secrets_dir=secrets_dir)
310338
return settings_factory()

app/main.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
from app.config.helpers import get_api_version, get_project_root
2525
from app.config.log_config import get_log_config
2626
from app.config.openapi import custom_openapi
27-
from app.config.settings import get_settings, unwrap_secret
27+
from app.config.settings import check_secrets_dir, get_settings, unwrap_secret
2828
from app.database.db import async_session
2929
from app.metrics.instrumentator import register_metrics
3030
from app.middleware.cache_logging import CacheLoggingMiddleware
@@ -73,6 +73,11 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[Any, None]:
7373
# Initialize loguru logging within the server process.
7474
get_log_config()
7575

76+
# Validate SECRETS_DIR configuration if set.
77+
for warning_msg in check_secrets_dir():
78+
logger.warning(warning_msg)
79+
loguru_logger.warning(warning_msg)
80+
7681
if "*" in cors_list:
7782
warning_msg = (
7883
"CORS_ORIGINS is set to '*', allowing any origin to access the "

tests/unit/test_validation.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@
1111
from cryptography.fernet import Fernet
1212
from pydantic import SecretStr
1313

14-
from app.config.settings import Settings, get_settings, unwrap_secret
14+
from app.config.settings import (
15+
Settings,
16+
check_secrets_dir,
17+
get_settings,
18+
unwrap_secret,
19+
)
1520

1621
if TYPE_CHECKING: # pragma: no cover
1722
from collections.abc import Callable
@@ -446,6 +451,7 @@ def test_get_settings_uses_secrets_dir_env_var(
446451
mock_settings = mocker.patch("app.config.settings.Settings")
447452
secrets_dir = "/var/run/app-secrets"
448453
monkeypatch.setenv("SECRETS_DIR", secrets_dir)
454+
mocker.patch("app.config.settings.Path.is_dir", return_value=True)
449455
get_settings.cache_clear()
450456

451457
settings = get_settings()
@@ -491,3 +497,31 @@ def test_weak_test_db_password_from_secrets_dir_rejected(
491497
),
492498
):
493499
build_settings(_env_file=None, _secrets_dir=tmp_path)
500+
501+
def test_secrets_dir_warns_when_not_found(self, monkeypatch) -> None:
502+
"""check_secrets_dir returns warning when path does not exist."""
503+
monkeypatch.setenv("SECRETS_DIR", "/no/such/secrets/dir")
504+
505+
warnings = check_secrets_dir()
506+
507+
assert len(warnings) == 1
508+
assert "does not exist" in warnings[0]
509+
510+
def test_secrets_dir_warns_when_writable(
511+
self, monkeypatch, tmp_path: Path
512+
) -> None:
513+
"""check_secrets_dir returns warning when dir is writable."""
514+
monkeypatch.setenv("SECRETS_DIR", str(tmp_path))
515+
516+
warnings = check_secrets_dir()
517+
518+
assert len(warnings) == 1
519+
assert "writable" in warnings[0]
520+
521+
def test_secrets_dir_no_warnings_when_unset(self, monkeypatch) -> None:
522+
"""check_secrets_dir returns empty list when SECRETS_DIR unset."""
523+
monkeypatch.delenv("SECRETS_DIR", raising=False)
524+
525+
warnings = check_secrets_dir()
526+
527+
assert warnings == []

0 commit comments

Comments
 (0)