From 0fb71509565fb84893d7a6ec59f2061b438eba8e Mon Sep 17 00:00:00 2001 From: tegwick Date: Sun, 6 Sep 2026 21:45:29 +0200 Subject: [PATCH] Read credentials from files, not only the environment Found by packaging the service for Railiance (rapp-canned-prompts). Every other rapp in the fleet mounts its database credential as a file; this service could only read CANNED_PROMPTS_DATABASE_URL from the environment, which would have put a database password into kubectl describe, into crash dumps, and in reach of anything able to read /proc. Adds CANNED_PROMPTS_DATABASE_URL_FILE and CANNED_PROMPTS_PUBLISH_TOKEN_FILE. A mounted secret stays a file. When both forms are set the file wins, because a rotated secret must take effect rather than be shadowed by a stale env var, and an unreadable secret file fails loudly rather than falling back to a value that may be older. Service tests 33 -> 36. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Bjefh8NUiEiahN4JLwoSKM Assistant: claude-code Assistant-Model: opus Assistant-Process: 388925@bnt-lap001 Assistant-Session: 3507023f-e0fd-4a1e-9d90-a0d4217d1502 --- service/README.md | 8 +++++ service/src/canned_prompts_service/api.py | 2 +- service/src/canned_prompts_service/auth.py | 5 +-- .../src/canned_prompts_service/settings.py | 32 ++++++++++++++++++- service/tests/test_health.py | 23 +++++++++++++ .../CANP-WP-0006-hosted-registry-service.md | 14 +++++++- 6 files changed, 79 insertions(+), 5 deletions(-) diff --git a/service/README.md b/service/README.md index f017404..073244a 100644 --- a/service/README.md +++ b/service/README.md @@ -27,6 +27,14 @@ export CANNED_PROMPTS_DATABASE_URL=postgresql+psycopg://... .venv/bin/uvicorn canned_prompts_service.api:create_app --factory ``` +Credentials may arrive as **files** rather than environment variables — +`CANNED_PROMPTS_DATABASE_URL_FILE` and `CANNED_PROMPTS_PUBLISH_TOKEN_FILE` — +which is how they are supplied in the cluster. An env var holding a password is +visible in `kubectl describe`, in crash dumps, and to anything that can read +`/proc`; a mounted secret should stay a file. When both forms are set the file +wins, because a rotated secret must take effect rather than be shadowed by a +stale env var. + `CANNED_PROMPTS_DATABASE_URL` has **no default**. A service that silently falls back to a local database when its real one is misconfigured is worse than one that refuses to start. diff --git a/service/src/canned_prompts_service/api.py b/service/src/canned_prompts_service/api.py index 58c22b5..eb0dace 100644 --- a/service/src/canned_prompts_service/api.py +++ b/service/src/canned_prompts_service/api.py @@ -24,7 +24,7 @@ from .settings import Settings, get_settings def create_app(settings: Settings | None = None, engine: Engine | None = None) -> FastAPI: settings = settings or get_settings() if engine is None and settings.configured: - engine = make_engine(settings.database_url) + engine = make_engine(settings.resolved_database_url) app = FastAPI(title="canned-prompts registry", version=__version__) app.state.settings = settings diff --git a/service/src/canned_prompts_service/auth.py b/service/src/canned_prompts_service/auth.py index f44c57e..0ba3acd 100644 --- a/service/src/canned_prompts_service/auth.py +++ b/service/src/canned_prompts_service/auth.py @@ -32,7 +32,8 @@ class PublishDenied(HTTPException): def require_publisher(request: Request, settings: Settings) -> str: """Return the publisher identity, or refuse.""" - if not settings.publish_token: + token = settings.resolved_publish_token + if not token: raise PublishDenied( status_code=503, detail=( @@ -49,6 +50,6 @@ def require_publisher(request: Request, settings: Settings) -> str: status_code=401, detail="publishing requires an Authorization: Bearer token" ) # compare_digest so a wrong token cannot be recovered by timing the reply. - if not hmac.compare_digest(credential, settings.publish_token): + if not hmac.compare_digest(credential, token): raise PublishDenied(status_code=403, detail="token not accepted") return settings.publisher_name diff --git a/service/src/canned_prompts_service/settings.py b/service/src/canned_prompts_service/settings.py index b6946a1..d374293 100644 --- a/service/src/canned_prompts_service/settings.py +++ b/service/src/canned_prompts_service/settings.py @@ -8,13 +8,20 @@ that refuses to start. from __future__ import annotations +from pathlib import Path + from pydantic_settings import BaseSettings, SettingsConfigDict class Settings(BaseSettings): model_config = SettingsConfigDict(env_prefix="CANNED_PROMPTS_", extra="ignore") + # Two ways in, and the file is the one to use in a cluster: an env var + # holding a password is visible in `kubectl describe`, in crash dumps, and + # to anything that can read /proc. The file form lets a mounted secret stay + # a file. database_url: str = "" + database_url_file: str = "" tenant: str = "default" service_name: str = "canned-prompts" @@ -24,9 +31,32 @@ class Settings(BaseSettings): publish_token: str = "" publisher_name: str = "operator" + publish_token_file: str = "" + + def _read_secret(self, path: str) -> str: + try: + return Path(path).read_text(encoding="utf-8").strip() + except OSError as exc: + raise RuntimeError(f"cannot read secret file {path}: {exc}") from exc + + @property + def resolved_database_url(self) -> str: + """The file wins when both are set: a mounted secret is the stronger + statement of intent, and silently preferring the env var would make a + rotated credential look like it had not taken effect.""" + if self.database_url_file: + return self._read_secret(self.database_url_file) + return self.database_url + + @property + def resolved_publish_token(self) -> str: + if self.publish_token_file: + return self._read_secret(self.publish_token_file) + return self.publish_token + @property def configured(self) -> bool: - return bool(self.database_url) + return bool(self.database_url or self.database_url_file) def get_settings() -> Settings: diff --git a/service/tests/test_health.py b/service/tests/test_health.py index 97a8f88..7bb197c 100644 --- a/service/tests/test_health.py +++ b/service/tests/test_health.py @@ -100,3 +100,26 @@ def test_settings_have_no_database_fallback() -> None: """Falling back to a local database when misconfigured hides the mistake.""" assert Settings().database_url == "" assert Settings().configured is False + + +def test_database_url_may_come_from_a_file(tmp_path: Path) -> None: + """A mounted secret should stay a file, not become an env var.""" + secret = tmp_path / "url" + secret.write_text("sqlite:///from-file.db\n", encoding="utf-8") + settings = Settings(database_url_file=str(secret)) + assert settings.configured is True + assert settings.resolved_database_url == "sqlite:///from-file.db" + + +def test_file_wins_over_env_when_both_are_set(tmp_path: Path) -> None: + """A rotated secret must take effect, not be shadowed by a stale env var.""" + secret = tmp_path / "url" + secret.write_text("sqlite:///from-file.db", encoding="utf-8") + settings = Settings(database_url="sqlite:///from-env.db", database_url_file=str(secret)) + assert settings.resolved_database_url == "sqlite:///from-file.db" + + +def test_unreadable_secret_file_fails_loudly(tmp_path: Path) -> None: + settings = Settings(database_url_file=str(tmp_path / "missing")) + with pytest.raises(RuntimeError, match="cannot read secret file"): + _ = settings.resolved_database_url diff --git a/workplans/CANP-WP-0006-hosted-registry-service.md b/workplans/CANP-WP-0006-hosted-registry-service.md index 2812207..9696d73 100644 --- a/workplans/CANP-WP-0006-hosted-registry-service.md +++ b/workplans/CANP-WP-0006-hosted-registry-service.md @@ -283,7 +283,19 @@ credentials, and outward-facing enough that it is not mine to take unasked. ## Follow-on: rapp-canned-prompts -Not a task in this workplan; recorded so the sequence is not lost. +`rapp-canned-prompts` now exists, is registered (agents / practice, prefix +`RCP-WP`), and carries its declaration, manifests and smoke tooling. +`RCP-WP-0002` holds the remaining steps. + +**Packaging it surfaced a gap in this service.** The deployment mounts +credentials as files, as the fleet's other rapps do, but the service only read +`CANNED_PROMPTS_DATABASE_URL` from the environment — which would have put a +database password into `kubectl describe`, crash dumps, and anything able to +read `/proc`. Added `CANNED_PROMPTS_DATABASE_URL_FILE` and +`CANNED_PROMPTS_PUBLISH_TOKEN_FILE`; the file form wins when both are set, so a +rotated secret takes effect instead of being shadowed. Service tests 33 → 36. + +Recorded so the sequence is not lost. 1. Publish the image to the fleet registry and capture its digest. 2. Create `rapp-canned-prompts` with `ownership_repo: canned-prompts`,