From 9751927d38b8c30d168aeeb1d19d6ca87cab31e8 Mon Sep 17 00:00:00 2001 From: tegwick Date: Sun, 23 Aug 2026 00:11:11 +0200 Subject: [PATCH] fix: adopt rotated database leases Assistant: codex Assistant-Model: gpt-5.6-sol Assistant-Session: 01a02b22-9638-76d2-bbff-b7ea1770b118 --- README.md | 4 ++ src/sbom_nexus/api.py | 14 ++++- src/sbom_nexus/database.py | 44 ++++++++++++- src/sbom_nexus/storage.py | 11 +++- tests/test_api.py | 1 + tests/test_database_rotation.py | 42 +++++++++++++ .../SBOM-WP-0004-database-lease-rotation.md | 63 +++++++++++++++++++ 7 files changed, 175 insertions(+), 4 deletions(-) create mode 100644 tests/test_database_rotation.py create mode 100644 workplans/SBOM-WP-0004-database-lease-rotation.md diff --git a/README.md b/README.md index 107c1a6..8cc0e4a 100644 --- a/README.md +++ b/README.md @@ -22,10 +22,14 @@ SQLite through `SBOM_NEXUS_DATABASE_PATH`; production uses `SBOM_NEXUS_DATABASE_URL_FILE=/var/run/secrets/.../url` and `make migrate`. The direct `SBOM_NEXUS_DATABASE_URL` variable remains available for disposable development environments; mounted secret files are preferred for production. +The runtime rereads the mounted file for every new database connection and +recycles pooled connections every 300 seconds by default, configurable with +`SBOM_NEXUS_DATABASE_POOL_RECYCLE_SECONDS`. ## Initial API surface - `GET /state/health` +- `GET /state/live` - `PUT /repositories/{repo_slug}` - `GET /sbom/catch-up?limit=3` - `POST /sbom/{repo_slug}/ingest` diff --git a/src/sbom_nexus/api.py b/src/sbom_nexus/api.py index dd14bbc..0eff001 100644 --- a/src/sbom_nexus/api.py +++ b/src/sbom_nexus/api.py @@ -207,9 +207,17 @@ def create_app(database_path: str | Path | None = None) -> FastAPI: version="0.1.0", description="SBOM capture, history, evaluation, and bounded catch-up service", ) - application.state.store = Store( + configured_target = ( database_path if database_path is not None else database_target("sbom-nexus.db") ) + application.state.store = Store( + configured_target, + database_url_file=( + os.getenv("SBOM_NEXUS_DATABASE_URL_FILE") + if database_path is None + else None + ), + ) if _auto_create(application.state.store): application.state.store.init_schema() @@ -219,6 +227,10 @@ def create_app(database_path: str | Path | None = None) -> FastAPI: store.health() return {"status": "ok", "store": "connected", "dialect": store.dialect} + @application.get("/state/live") + def live() -> dict[str, str]: + return {"status": "ok"} + @application.put("/repositories/{repo_slug}") def upsert_repository( repo_slug: str, body: RepositoryUpsert, request: Request diff --git a/src/sbom_nexus/database.py b/src/sbom_nexus/database.py index c7e4b08..a5cd60e 100644 --- a/src/sbom_nexus/database.py +++ b/src/sbom_nexus/database.py @@ -2,8 +2,10 @@ from __future__ import annotations +import os from pathlib import Path +import psycopg from sqlalchemy import ( JSON, Boolean, @@ -128,9 +130,49 @@ def database_url(value: str | Path) -> str: return text -def create_database_engine(value: str | Path) -> Engine: +def _dynamic_connection_factory(url_file: Path): + def connect(): + try: + value = url_file.read_text(encoding="utf-8").strip() + except OSError as exc: + raise RuntimeError("Unable to read mounted database URL") from exc + if not value: + raise RuntimeError("Mounted database URL is empty") + url = database_url(value) + if not url.startswith("postgresql+psycopg://"): + raise RuntimeError("Mounted database URL must use PostgreSQL") + return psycopg.connect( + url.replace("postgresql+psycopg://", "postgresql://", 1) + ) + + return connect + + +def create_database_engine( + value: str | Path, *, database_url_file: str | Path | None = None +) -> Engine: url = database_url(value) options: dict[str, object] = {"pool_pre_ping": True} + if database_url_file: + if not url.startswith("postgresql+psycopg://"): + raise RuntimeError("Dynamic database URL files require PostgreSQL") + try: + recycle_seconds = int( + os.getenv("SBOM_NEXUS_DATABASE_POOL_RECYCLE_SECONDS", "300") + ) + except ValueError as exc: + raise RuntimeError( + "SBOM_NEXUS_DATABASE_POOL_RECYCLE_SECONDS must be an integer" + ) from exc + if recycle_seconds < 1: + raise RuntimeError( + "SBOM_NEXUS_DATABASE_POOL_RECYCLE_SECONDS must be positive" + ) + options["creator"] = _dynamic_connection_factory(Path(database_url_file)) + options["pool_recycle"] = recycle_seconds + # Keep only the dialect in SQLAlchemy's URL; the creator rereads the + # mounted value without retaining credentials in engine repr/logging. + url = "postgresql+psycopg://" if url.startswith("sqlite:"): options["connect_args"] = {"check_same_thread": False} engine = create_engine(url, **options) diff --git a/src/sbom_nexus/storage.py b/src/sbom_nexus/storage.py index 011b49d..8506dbf 100644 --- a/src/sbom_nexus/storage.py +++ b/src/sbom_nexus/storage.py @@ -50,8 +50,15 @@ def datetime_text(value: datetime | str | None) -> str | None: class Store: - def __init__(self, database_target: str | Path) -> None: - self.engine = create_database_engine(database_target) + def __init__( + self, + database_target: str | Path, + *, + database_url_file: str | Path | None = None, + ) -> None: + self.engine = create_database_engine( + database_target, database_url_file=database_url_file + ) @property def dialect(self) -> str: diff --git a/tests/test_api.py b/tests/test_api.py index 50b1d37..4f30992 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -43,6 +43,7 @@ def test_health_and_legacy_ingest_query_and_licence_report(tmp_path: Path) -> No "store": "connected", "dialect": "sqlite", } + assert client.get("/state/live").json() == {"status": "ok"} register(client, "demo") response = client.post( diff --git a/tests/test_database_rotation.py b/tests/test_database_rotation.py new file mode 100644 index 0000000..742e874 --- /dev/null +++ b/tests/test_database_rotation.py @@ -0,0 +1,42 @@ +from __future__ import annotations + +from pathlib import Path + +from sbom_nexus import database + + +def test_dynamic_connection_factory_rereads_mounted_url(monkeypatch, tmp_path: Path) -> None: + secret = tmp_path / "url" + observed: list[str] = [] + + def fake_connect(dsn: str): + observed.append(dsn) + return object() + + monkeypatch.setattr(database.psycopg, "connect", fake_connect) + factory = database._dynamic_connection_factory(secret) + secret.write_text("postgresql://first@db/sbom\n", encoding="utf-8") + factory() + secret.write_text("postgresql://second@db/sbom\n", encoding="utf-8") + factory() + + assert observed == [ + "postgresql://first@db/sbom", + "postgresql://second@db/sbom", + ] + + +def test_dynamic_engine_keeps_credential_out_of_engine_url( + monkeypatch, tmp_path: Path +) -> None: + secret = tmp_path / "url" + secret.write_text("postgresql://username:password@db/sbom\n", encoding="utf-8") + monkeypatch.setenv("SBOM_NEXUS_DATABASE_POOL_RECYCLE_SECONDS", "60") + + engine = database.create_database_engine( + secret.read_text().strip(), database_url_file=secret + ) + + assert "username" not in str(engine.url) + assert "password" not in str(engine.url) + assert engine.pool._recycle == 60 diff --git a/workplans/SBOM-WP-0004-database-lease-rotation.md b/workplans/SBOM-WP-0004-database-lease-rotation.md new file mode 100644 index 0000000..52daeae --- /dev/null +++ b/workplans/SBOM-WP-0004-database-lease-rotation.md @@ -0,0 +1,63 @@ +--- +id: SBOM-WP-0004 +type: workplan +title: "Adopt rotated database leases without liveness restarts" +domain: infotech +repo: sbom-nexus +status: active +owner: codex +topic_slug: infotech +created: "2026-08-23" +updated: "2026-08-23" +quality_dor: DoR-Ok +quality_dor_at: "2026-08-23" +quality_dor_by: codex +quality_dor_note: "CUST-IN-0014 supplies exact 30-minute expiry/restart evidence, mounted Secret rotation behavior, owner boundaries, value-safety constraints, and live acceptance criteria." +origin: residual +origin_ref: CUST-IN-0014 +related: + - RAPP-SBOM-NEXUS-WP-0003 +--- + +# Adopt rotated database leases without liveness restarts + +## Reread the mounted DSN on new pool connections + +```task +id: SBOM-WP-0004-T01 +status: done +priority: high +``` + +Use a value-safe SQLAlchemy connection creator that rereads the mounted URL +for every new DBAPI connection. Recycle pooled connections before the current +30-minute lease expires and preserve `pool_pre_ping` so revoked sessions are +replaced with the current mounted credential. + +Completed with tests proving two connection attempts observe two file values +and that the engine URL contains neither username nor password. + +## Separate liveness from database readiness + +```task +id: SBOM-WP-0004-T02 +status: done +priority: high +``` + +Expose process-only `/state/live` while retaining the database-backed +`/state/health` readiness contract. The package must move only liveness to the +new route so a transient credential handoff removes traffic but does not ask +Kubernetes to restart an otherwise healthy process. + +## Prove one complete live lease rotation + +```task +id: SBOM-WP-0004-T03 +status: wait +priority: high +``` + +Promote the shared new image/package digest, observe at least one complete +database lease rotation, and require continuous process uptime, recovered +readiness, no liveness-driven restart, and no credential values in logs.