feat(crypto): fail loudly on unpersistable secret key; flag undecryptable secrets
Follow-up to the incident where an unwritable /data made Steward silently mint a fresh ephemeral secret key on every boot, orphaning all encrypted secrets — only discovered when an Ansible run failed. Make both failure modes loud and visible. - config._resolve_secret_key: if a new key must be generated but can't be persisted (no STEWARD_SECRET_KEY, unwritable /data), raise RuntimeError and refuse to start, with an actionable fix (set STEWARD_SECRET_KEY, or make /data writable by uid 1000). Also raises a clear error if an existing key file can't be read. First-run on a writable volume still generates + persists normally. - core.settings: detect secrets stored as ciphertext that won't decrypt with the current key (scan_undecryptable_secrets at startup; _is_undecryptable helper). Cached in memory; set_setting discards a key on a fresh write so the banner clears without a restart. - app.py: run the scan after migrate_plaintext_secrets, log a warning, and inject undecryptable_secrets into the template context. - base.html: admin banner naming which secrets to re-enter, each linked to its settings tab. - compose.deploy.yml: document the uid-1000 /data write requirement and the STEWARD_SECRET_KEY option for multi-node Swarm. - Tests: secret-key resolver (reuse existing file, generate when writable, raise when unpersistable) and the undecryptable-secret detection helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Jg27rgypiW2efULXJDtMC
This commit is contained in:
+11
-1
@@ -22,12 +22,22 @@ services:
|
|||||||
# /data holds the auto-generated secret key, third-party plugins, and the
|
# /data holds the auto-generated secret key, third-party plugins, and the
|
||||||
# Ansible playbook cache — all of which must survive image updates. No
|
# Ansible playbook cache — all of which must survive image updates. No
|
||||||
# source bind-mounts: core + first-party plugins live inside the image.
|
# source bind-mounts: core + first-party plugins live inside the image.
|
||||||
|
#
|
||||||
|
# The container runs as the unprivileged 'app' user (uid 1000), so /data
|
||||||
|
# MUST be writable by uid 1000. A named docker volume (below) gets that
|
||||||
|
# automatically. If you bind-mount a host/NFS path instead, chown it to
|
||||||
|
# 1000:1000 (and mind NFS root_squash) — otherwise the app can't persist
|
||||||
|
# its secret key, mints a fresh one each boot, and every encrypted secret
|
||||||
|
# (managed SSH key, SMTP/OIDC/LDAP creds) becomes unrecoverable. The app
|
||||||
|
# now refuses to start in that state rather than silently degrade.
|
||||||
- app_data:/data
|
- app_data:/data
|
||||||
environment:
|
environment:
|
||||||
- STEWARD_DATABASE_URL=postgresql+asyncpg://steward:${POSTGRES_PASSWORD:-steward}@db/steward
|
- STEWARD_DATABASE_URL=postgresql+asyncpg://steward:${POSTGRES_PASSWORD:-steward}@db/steward
|
||||||
# STEWARD_SECRET_KEY is intentionally absent — the app generates a random
|
# STEWARD_SECRET_KEY is intentionally absent — the app generates a random
|
||||||
# key on first boot and persists it to /data/secret.key, which the app_data
|
# key on first boot and persists it to /data/secret.key, which the app_data
|
||||||
# volume keeps across image updates.
|
# volume keeps across image updates. On multi-node Swarm with a shared bind
|
||||||
|
# mount, pin STEWARD_SECRET_KEY (env or a docker secret) instead, so the key
|
||||||
|
# never depends on filesystem ownership being correct on every node.
|
||||||
depends_on:
|
depends_on:
|
||||||
db:
|
db:
|
||||||
condition: service_healthy
|
condition: service_healthy
|
||||||
|
|||||||
+16
-1
@@ -1,11 +1,14 @@
|
|||||||
# steward/app.py
|
# steward/app.py
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
import asyncio
|
import asyncio
|
||||||
|
import logging
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from quart import Quart, render_template
|
from quart import Quart, render_template
|
||||||
from .config import load_bootstrap
|
from .config import load_bootstrap
|
||||||
from .database import init_db
|
from .database import init_db
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
def create_app(
|
def create_app(
|
||||||
config_path: Path | str | None = None,
|
config_path: Path | str | None = None,
|
||||||
@@ -50,9 +53,17 @@ def create_app(
|
|||||||
# ── 3b. Encryption-at-rest: bind the encryptor, then encrypt legacy secrets ─
|
# ── 3b. Encryption-at-rest: bind the encryptor, then encrypt legacy secrets ─
|
||||||
if not testing:
|
if not testing:
|
||||||
from .core.crypto import init_crypto
|
from .core.crypto import init_crypto
|
||||||
from .core.settings import migrate_plaintext_secrets
|
from .core.settings import migrate_plaintext_secrets, scan_undecryptable_secrets
|
||||||
init_crypto(app.config["SECRET_KEY"])
|
init_crypto(app.config["SECRET_KEY"])
|
||||||
migrate_plaintext_secrets(app.config["DATABASE_URL"])
|
migrate_plaintext_secrets(app.config["DATABASE_URL"])
|
||||||
|
# Flag any secrets sealed under a now-lost key (see the admin banner).
|
||||||
|
undecryptable = scan_undecryptable_secrets(app.config["DATABASE_URL"])
|
||||||
|
if undecryptable:
|
||||||
|
logger.warning(
|
||||||
|
"%d stored secret(s) cannot be decrypted with the current app key "
|
||||||
|
"(re-enter them in Settings): %s",
|
||||||
|
len(undecryptable), ", ".join(undecryptable),
|
||||||
|
)
|
||||||
|
|
||||||
# ── 4. Load all settings from DB → populate app.config ────────────────────
|
# ── 4. Load all settings from DB → populate app.config ────────────────────
|
||||||
if not testing:
|
if not testing:
|
||||||
@@ -167,6 +178,7 @@ def create_app(
|
|||||||
@app.context_processor
|
@app.context_processor
|
||||||
def _inject_plugin_failures():
|
def _inject_plugin_failures():
|
||||||
from .core.plugin_manager import get_plugin_failures
|
from .core.plugin_manager import get_plugin_failures
|
||||||
|
from .core.settings import get_undecryptable_secrets
|
||||||
# enabled_plugins lets templates gate cross-plugin embeds (e.g. the
|
# enabled_plugins lets templates gate cross-plugin embeds (e.g. the
|
||||||
# Hosts hub only fetches the docker fragment when docker is enabled),
|
# Hosts hub only fetches the docker fragment when docker is enabled),
|
||||||
# avoiding a 404 to a route whose blueprint isn't registered.
|
# avoiding a 404 to a route whose blueprint isn't registered.
|
||||||
@@ -177,6 +189,9 @@ def create_app(
|
|||||||
return {
|
return {
|
||||||
"plugin_failures": get_plugin_failures(),
|
"plugin_failures": get_plugin_failures(),
|
||||||
"enabled_plugins": enabled_plugins,
|
"enabled_plugins": enabled_plugins,
|
||||||
|
# Read from an in-memory cache (no DB hit per render); kept current by
|
||||||
|
# the startup scan + set_setting's discard-on-write.
|
||||||
|
"undecryptable_secrets": get_undecryptable_secrets(),
|
||||||
}
|
}
|
||||||
|
|
||||||
# ── 11. Share-token middleware ─────────────────────────────────────────────
|
# ── 11. Share-token middleware ─────────────────────────────────────────────
|
||||||
|
|||||||
+26
-7
@@ -75,13 +75,27 @@ def load_bootstrap(config_path: Path | str | None = None) -> dict[str, Any]:
|
|||||||
|
|
||||||
|
|
||||||
def _resolve_secret_key(raw: dict) -> str:
|
def _resolve_secret_key(raw: dict) -> str:
|
||||||
"""Resolve secret_key: env var → file → auto-generate."""
|
"""Resolve secret_key: env var → file → auto-generate.
|
||||||
|
|
||||||
|
Refuses to start if a new key must be generated but cannot be persisted: an
|
||||||
|
ephemeral key changes on every restart, which silently renders every
|
||||||
|
encrypted secret (managed SSH key, SMTP/OIDC/LDAP credentials) unrecoverable.
|
||||||
|
Failing loudly with a fix beats limping along and losing data on the next
|
||||||
|
boot — exactly the footgun that bit the vdnt-docker02 deployment.
|
||||||
|
"""
|
||||||
from_env = _env("SECRET_KEY") or raw.get("secret_key")
|
from_env = _env("SECRET_KEY") or raw.get("secret_key")
|
||||||
if from_env:
|
if from_env:
|
||||||
return from_env
|
return from_env
|
||||||
|
|
||||||
if _SECRET_KEY_FILE.exists():
|
if _SECRET_KEY_FILE.exists():
|
||||||
|
try:
|
||||||
key = _SECRET_KEY_FILE.read_text().strip()
|
key = _SECRET_KEY_FILE.read_text().strip()
|
||||||
|
except OSError as exc:
|
||||||
|
raise RuntimeError(
|
||||||
|
f"App secret key file {_SECRET_KEY_FILE} exists but cannot be read "
|
||||||
|
f"({exc}). Make it readable by the container user (uid 1000), or set "
|
||||||
|
f"STEWARD_SECRET_KEY."
|
||||||
|
) from exc
|
||||||
if key:
|
if key:
|
||||||
return key
|
return key
|
||||||
|
|
||||||
@@ -89,11 +103,16 @@ def _resolve_secret_key(raw: dict) -> str:
|
|||||||
try:
|
try:
|
||||||
_SECRET_KEY_FILE.parent.mkdir(parents=True, exist_ok=True)
|
_SECRET_KEY_FILE.parent.mkdir(parents=True, exist_ok=True)
|
||||||
_SECRET_KEY_FILE.write_text(key)
|
_SECRET_KEY_FILE.write_text(key)
|
||||||
logger.info("Generated new secret key and saved to %s", _SECRET_KEY_FILE)
|
|
||||||
except OSError as exc:
|
except OSError as exc:
|
||||||
logger.warning(
|
raise RuntimeError(
|
||||||
"Could not write secret key to %s (%s). "
|
f"Generated a new app secret key but could not persist it to "
|
||||||
"Key will not persist across restarts.",
|
f"{_SECRET_KEY_FILE} ({exc}). An ephemeral key changes on every restart, "
|
||||||
_SECRET_KEY_FILE, exc,
|
f"which makes all encrypted secrets (managed SSH key, SMTP/OIDC/LDAP "
|
||||||
)
|
f"credentials) unrecoverable. Fix one of:\n"
|
||||||
|
f" • set STEWARD_SECRET_KEY to a stable value "
|
||||||
|
f"(recommended for Swarm / multi-node), or\n"
|
||||||
|
f" • make {_SECRET_KEY_FILE.parent} writable by the container user "
|
||||||
|
f"(uid 1000)."
|
||||||
|
) from exc
|
||||||
|
logger.info("Generated new secret key and saved to %s", _SECRET_KEY_FILE)
|
||||||
return key
|
return key
|
||||||
|
|||||||
@@ -150,6 +150,34 @@ def _decode(value: Any, key: str = "") -> Any:
|
|||||||
return decrypt_secret(value, context=key) if is_encrypted(value) else value
|
return decrypt_secret(value, context=key) if is_encrypted(value) else value
|
||||||
|
|
||||||
|
|
||||||
|
# Secret settings that are stored as ciphertext but won't decrypt with the
|
||||||
|
# current app key (key rotated or lost). Surfaced as an admin banner so the
|
||||||
|
# operator knows precisely which secrets to re-enter — instead of finding out
|
||||||
|
# only when something that uses one fails. Refreshed at startup
|
||||||
|
# (scan_undecryptable_secrets) and kept live: re-entering a secret clears it
|
||||||
|
# without a restart (set_setting discards it on a fresh write).
|
||||||
|
_undecryptable_secrets: set[str] = set()
|
||||||
|
|
||||||
|
|
||||||
|
def get_undecryptable_secrets() -> list[str]:
|
||||||
|
"""Sorted keys whose stored ciphertext won't decrypt (for the UI banner)."""
|
||||||
|
return sorted(_undecryptable_secrets)
|
||||||
|
|
||||||
|
|
||||||
|
def _is_undecryptable(stored: Any, key: str = "") -> bool:
|
||||||
|
"""True iff `stored` is an encrypted token that fails to decrypt.
|
||||||
|
|
||||||
|
A failed decrypt returns the ciphertext unchanged (still enc-prefixed), so a
|
||||||
|
value that is still encrypted after a decrypt attempt is undecryptable.
|
||||||
|
"""
|
||||||
|
from steward.core.crypto import decrypt_secret, is_encrypted
|
||||||
|
return (
|
||||||
|
isinstance(stored, str)
|
||||||
|
and is_encrypted(stored)
|
||||||
|
and is_encrypted(decrypt_secret(stored, context=key))
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
# ─────────────────────────────────────────────────────────────────────────────
|
# ─────────────────────────────────────────────────────────────────────────────
|
||||||
# Async helpers (use inside request handlers / scheduled tasks)
|
# Async helpers (use inside request handlers / scheduled tasks)
|
||||||
# ─────────────────────────────────────────────────────────────────────────────
|
# ─────────────────────────────────────────────────────────────────────────────
|
||||||
@@ -182,6 +210,12 @@ async def set_setting(session: AsyncSession, key: str, value: Any) -> None:
|
|||||||
row.value_json = json.dumps(to_store)
|
row.value_json = json.dumps(to_store)
|
||||||
row.updated_at = now
|
row.updated_at = now
|
||||||
|
|
||||||
|
# A fresh write of a secret is encrypted with the current key (or cleared to
|
||||||
|
# plaintext), so it's decryptable now — drop any stale "undecryptable" flag
|
||||||
|
# so the banner clears without a restart.
|
||||||
|
if key in SECRET_KEYS:
|
||||||
|
_undecryptable_secrets.discard(key)
|
||||||
|
|
||||||
|
|
||||||
async def get_all_settings(session: AsyncSession) -> dict[str, Any]:
|
async def get_all_settings(session: AsyncSession) -> dict[str, Any]:
|
||||||
"""Return flat key→value dict with defaults filled in for missing keys."""
|
"""Return flat key→value dict with defaults filled in for missing keys."""
|
||||||
@@ -359,6 +393,37 @@ def migrate_plaintext_secrets(db_url: str) -> int:
|
|||||||
return asyncio.run(_run())
|
return asyncio.run(_run())
|
||||||
|
|
||||||
|
|
||||||
|
def scan_undecryptable_secrets(db_url: str) -> list[str]:
|
||||||
|
"""Populate the undecryptable-secrets cache from the DB; return the keys found.
|
||||||
|
|
||||||
|
Run once at startup, AFTER migrate_plaintext_secrets and init_crypto: any row
|
||||||
|
that's encrypted but won't decrypt with the current key was sealed under a
|
||||||
|
different (lost/rotated) key and must be re-entered. Surfaced via the admin
|
||||||
|
banner (get_undecryptable_secrets).
|
||||||
|
"""
|
||||||
|
async def _run() -> list[str]:
|
||||||
|
from sqlalchemy.ext.asyncio import create_async_engine, async_sessionmaker
|
||||||
|
engine = create_async_engine(db_url, echo=False)
|
||||||
|
factory = async_sessionmaker(engine, expire_on_commit=False)
|
||||||
|
bad: list[str] = []
|
||||||
|
try:
|
||||||
|
async with factory() as session:
|
||||||
|
result = await session.execute(
|
||||||
|
select(AppSetting).where(AppSetting.key.in_(SECRET_KEYS))
|
||||||
|
)
|
||||||
|
for row in result.scalars():
|
||||||
|
if _is_undecryptable(json.loads(row.value_json), row.key):
|
||||||
|
bad.append(row.key)
|
||||||
|
finally:
|
||||||
|
await engine.dispose()
|
||||||
|
return bad
|
||||||
|
|
||||||
|
global _undecryptable_secrets
|
||||||
|
found = asyncio.run(_run())
|
||||||
|
_undecryptable_secrets = set(found)
|
||||||
|
return sorted(found)
|
||||||
|
|
||||||
|
|
||||||
# ─────────────────────────────────────────────────────────────────────────────
|
# ─────────────────────────────────────────────────────────────────────────────
|
||||||
# External URL helper
|
# External URL helper
|
||||||
# ─────────────────────────────────────────────────────────────────────────────
|
# ─────────────────────────────────────────────────────────────────────────────
|
||||||
|
|||||||
@@ -280,6 +280,34 @@ function setTimeRange(val) {
|
|||||||
</span>
|
</span>
|
||||||
</div>
|
</div>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
{% if undecryptable_secrets and session.user_role == 'admin' %}
|
||||||
|
{% set _sec_tab = {
|
||||||
|
'smtp.password': '/settings/notifications/',
|
||||||
|
'oidc.client_secret': '/settings/auth/',
|
||||||
|
'ldap.bind_password': '/settings/auth/',
|
||||||
|
'ansible.ssh_private_key': '/settings/ansible/',
|
||||||
|
'ansible.become_password': '/settings/ansible/',
|
||||||
|
'ansible.vault_password': '/settings/ansible/',
|
||||||
|
} %}
|
||||||
|
<div style="background:color-mix(in srgb,var(--red) 12%,var(--bg-elevated));
|
||||||
|
border:1px solid color-mix(in srgb,var(--red) 35%,var(--border));
|
||||||
|
border-radius:6px;padding:0.6rem 1rem;margin-bottom:1rem;
|
||||||
|
font-size:0.84rem;display:flex;align-items:flex-start;gap:0.6rem;">
|
||||||
|
<span style="color:var(--red);flex-shrink:0;">⚠</span>
|
||||||
|
<span>
|
||||||
|
<strong style="color:var(--text);">
|
||||||
|
{{ undecryptable_secrets | length }} stored secret{{ 's' if undecryptable_secrets | length != 1 }}
|
||||||
|
can’t be decrypted
|
||||||
|
</strong>
|
||||||
|
— the app secret key changed, so {{ 'they' if undecryptable_secrets | length != 1 else 'it' }}
|
||||||
|
must be re-entered:
|
||||||
|
{% for k in undecryptable_secrets -%}
|
||||||
|
{%- if _sec_tab.get(k) %}<a href="{{ _sec_tab[k] }}" style="color:var(--accent);">{{ k }}</a>
|
||||||
|
{%- else %}<code>{{ k }}</code>{% endif %}{% if not loop.last %}, {% endif %}
|
||||||
|
{%- endfor %}.
|
||||||
|
</span>
|
||||||
|
</div>
|
||||||
|
{% endif %}
|
||||||
{% if error %}
|
{% if error %}
|
||||||
<div class="alert alert-error">{{ error }}</div>
|
<div class="alert alert-error">{{ error }}</div>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
|||||||
@@ -0,0 +1,29 @@
|
|||||||
|
"""Unit tests for undecryptable-secret detection (the admin-banner source)."""
|
||||||
|
from steward.core.crypto import encrypt_secret, init_crypto
|
||||||
|
from steward.core.settings import _is_undecryptable, get_undecryptable_secrets
|
||||||
|
|
||||||
|
|
||||||
|
def test_is_undecryptable_false_for_current_key():
|
||||||
|
init_crypto("key-A")
|
||||||
|
tok = encrypt_secret("hunter2")
|
||||||
|
assert _is_undecryptable(tok, "smtp.password") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_is_undecryptable_true_after_key_rotation():
|
||||||
|
init_crypto("key-A")
|
||||||
|
tok = encrypt_secret("hunter2") # sealed under key-A
|
||||||
|
init_crypto("key-B") # key changed/lost
|
||||||
|
assert _is_undecryptable(tok, "smtp.password") is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_is_undecryptable_ignores_plaintext_and_empty():
|
||||||
|
init_crypto("key-A")
|
||||||
|
assert _is_undecryptable("", "smtp.password") is False
|
||||||
|
assert _is_undecryptable("plain-value", "smtp.password") is False
|
||||||
|
assert _is_undecryptable(None, "smtp.password") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_undecryptable_secrets_returns_sorted_list():
|
||||||
|
# Default (nothing scanned) is an empty list; the accessor is always safe to
|
||||||
|
# call from the template context processor.
|
||||||
|
assert isinstance(get_undecryptable_secrets(), list)
|
||||||
+30
-1
@@ -1,6 +1,6 @@
|
|||||||
import textwrap
|
import textwrap
|
||||||
import pytest
|
import pytest
|
||||||
from steward.config import load_bootstrap
|
from steward.config import load_bootstrap, _resolve_secret_key
|
||||||
|
|
||||||
|
|
||||||
def test_load_bootstrap_from_yaml(tmp_path):
|
def test_load_bootstrap_from_yaml(tmp_path):
|
||||||
@@ -36,3 +36,32 @@ def test_missing_database_url_raises(tmp_path):
|
|||||||
cfg_file.write_text("secret_key: s\n")
|
cfg_file.write_text("secret_key: s\n")
|
||||||
with pytest.raises(ValueError, match="Database URL is required"):
|
with pytest.raises(ValueError, match="Database URL is required"):
|
||||||
load_bootstrap(cfg_file)
|
load_bootstrap(cfg_file)
|
||||||
|
|
||||||
|
|
||||||
|
# ── secret key resolution ──────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
def test_secret_key_existing_file_is_reused(tmp_path, monkeypatch):
|
||||||
|
key_file = tmp_path / "secret.key"
|
||||||
|
key_file.write_text("persisted-key\n")
|
||||||
|
monkeypatch.setattr("steward.config._SECRET_KEY_FILE", key_file)
|
||||||
|
monkeypatch.delenv("STEWARD_SECRET_KEY", raising=False)
|
||||||
|
assert _resolve_secret_key({}) == "persisted-key"
|
||||||
|
|
||||||
|
|
||||||
|
def test_secret_key_generated_and_persisted_when_writable(tmp_path, monkeypatch):
|
||||||
|
key_file = tmp_path / "sub" / "secret.key" # parent doesn't exist yet
|
||||||
|
monkeypatch.setattr("steward.config._SECRET_KEY_FILE", key_file)
|
||||||
|
monkeypatch.delenv("STEWARD_SECRET_KEY", raising=False)
|
||||||
|
key = _resolve_secret_key({})
|
||||||
|
assert key and key_file.read_text().strip() == key
|
||||||
|
|
||||||
|
|
||||||
|
def test_secret_key_unpersistable_raises_instead_of_ephemeral(tmp_path, monkeypatch):
|
||||||
|
# A file sits where a directory is needed, so mkdir/write fails → must raise
|
||||||
|
# (an ephemeral key would silently orphan every encrypted secret).
|
||||||
|
blocker = tmp_path / "blocker"
|
||||||
|
blocker.write_text("not a dir")
|
||||||
|
monkeypatch.setattr("steward.config._SECRET_KEY_FILE", blocker / "secret.key")
|
||||||
|
monkeypatch.delenv("STEWARD_SECRET_KEY", raising=False)
|
||||||
|
with pytest.raises(RuntimeError, match="could not persist"):
|
||||||
|
_resolve_secret_key({})
|
||||||
|
|||||||
Reference in New Issue
Block a user