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 <noreply@anthropic.com> 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
This commit is contained in:
parent
cb133f3673
commit
0fb7150956
6 changed files with 79 additions and 5 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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`,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue