mirror of
https://github.com/Routstr/routstr-core.git
synced 2026-07-30 15:26:14 +00:00
fix(settings): keep upstream_api_key in the settings blob
upstream_api_key was added to SECRET_FIELDS, so it was stripped from every blob write — but unlike nsec, nothing migrates it into encrypted storage. A node carrying it only in the DB blob would load it into memory once, rewrite the blob without it, and lose it on the next restart, breaking upstream auth. It is node-scoped config that really belongs on a provider, not a vault secret, and it has no encrypted home yet. Remove it from SECRET_FIELDS so it stays in the blob exactly as before; redaction on read and ignore-on-write in the admin settings endpoint are unchanged. Encrypting it is follow-up work. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
14748c28fb
commit
afcb3f7cda
@@ -135,11 +135,14 @@ def _normalize_settings_data(data: dict[str, Any]) -> dict[str, Any]:
|
||||
|
||||
# Secrets are credentials, not config: they live in the encrypted/hashed Secret
|
||||
# store (and decrypted in-memory for runtime use), never in the persisted
|
||||
# settings blob. ``admin_password`` is gone from the model entirely;
|
||||
# ``nsec``/``upstream_api_key`` remain live fields but are stripped from every
|
||||
# blob write so they are never written back to plaintext. See
|
||||
# ``bootstrap_secrets`` and ``routstr.core.vault``.
|
||||
SECRET_FIELDS = frozenset({"admin_password", "nsec", "upstream_api_key"})
|
||||
# settings blob. ``admin_password`` is gone from the model entirely; ``nsec``
|
||||
# remains a live field but is stripped from every blob write so it is never
|
||||
# written back to plaintext. ``upstream_api_key`` is intentionally *not* here:
|
||||
# it has no encrypted home yet (it is node-scoped today but really belongs on a
|
||||
# provider), so stripping it would lose it on the next restart. It stays in the
|
||||
# blob as before; encrypting it is follow-up work. See ``bootstrap_secrets`` and
|
||||
# ``routstr.core.vault``.
|
||||
SECRET_FIELDS = frozenset({"admin_password", "nsec"})
|
||||
|
||||
|
||||
def _strip_secret_fields(data: dict[str, Any]) -> dict[str, Any]:
|
||||
@@ -350,7 +353,7 @@ class SettingsService:
|
||||
await db_session.commit()
|
||||
|
||||
# Update the existing instance in-place for all live importers
|
||||
# (keeps the decrypted nsec/upstream_api_key live in memory).
|
||||
# (keeps the decrypted nsec live in memory).
|
||||
_apply_to_live_settings(merged_dict)
|
||||
cls._current = settings
|
||||
return cls._current
|
||||
|
||||
@@ -2,9 +2,9 @@
|
||||
|
||||
``admin_password`` is no longer a settings field (it lives only as a one-way
|
||||
hash in the Secret store), so it must never appear in the GET/PATCH payloads.
|
||||
``nsec`` and ``upstream_api_key`` remain live in-memory runtime values but are
|
||||
redacted on read and ignored on write — they cannot be set through the general
|
||||
settings endpoint, only through their dedicated rotation paths.
|
||||
``nsec`` (in-memory at runtime) and ``upstream_api_key`` (still in the settings
|
||||
blob) are both redacted on read and ignored on write — they cannot be set
|
||||
through the general settings endpoint, only through their dedicated paths.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
+36
-12
@@ -138,7 +138,8 @@ def test_settings_model_drops_admin_password_field() -> None:
|
||||
# admin_password now lives only as a one-way hash in the Secret store; it is
|
||||
# no longer a settings field at all.
|
||||
assert "admin_password" not in Settings.__fields__
|
||||
# nsec and upstream_api_key remain runtime values held in memory.
|
||||
# nsec remains a runtime value held in memory; upstream_api_key is ordinary
|
||||
# config that still lives in the persisted blob.
|
||||
assert "nsec" in Settings.__fields__
|
||||
assert "upstream_api_key" in Settings.__fields__
|
||||
|
||||
@@ -148,28 +149,53 @@ async def test_secret_fields_kept_in_memory_but_not_persisted(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setenv("NSEC", NSEC_HEX)
|
||||
monkeypatch.setenv("UPSTREAM_API_KEY", "sk-upstream")
|
||||
# Reset the live globals so monkeypatch reverts them after the test.
|
||||
monkeypatch.setattr(settings, "nsec", "")
|
||||
monkeypatch.setattr(settings, "upstream_api_key", "")
|
||||
|
||||
engine = create_async_engine("sqlite+aiosqlite:///:memory:")
|
||||
async with AsyncSession(engine, expire_on_commit=False) as session:
|
||||
s = await SettingsService.initialize(session)
|
||||
|
||||
# Runtime consumers still see the live secret values.
|
||||
# Runtime consumers still see the live secret value.
|
||||
assert s.nsec == NSEC_HEX
|
||||
assert s.upstream_api_key == "sk-upstream"
|
||||
|
||||
# ...but they are never written to the settings blob.
|
||||
# ...but it is never written to the settings blob.
|
||||
blob = await _read_settings_blob(session)
|
||||
assert "nsec" not in blob
|
||||
assert "upstream_api_key" not in blob
|
||||
assert "admin_password" not in blob
|
||||
# Non-secret derived/public values are still persisted.
|
||||
assert blob["npub"] == s.npub
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_upstream_api_key_survives_persistence(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
# upstream_api_key is provider-scoped config, not a vault secret: it has no
|
||||
# encrypted home yet, so it must stay in the settings blob. Stripping it
|
||||
# would load it once, rewrite the blob without it, and lose it on the next
|
||||
# restart. Guard the on-disk survival path: blob-only value, no env.
|
||||
monkeypatch.delenv("UPSTREAM_API_KEY", raising=False)
|
||||
monkeypatch.setattr(settings, "upstream_api_key", "")
|
||||
|
||||
engine = create_async_engine("sqlite+aiosqlite:///:memory:")
|
||||
async with AsyncSession(engine, expire_on_commit=False) as session:
|
||||
await SettingsService.initialize(session)
|
||||
await session.exec( # type: ignore
|
||||
text("UPDATE settings SET data = :d WHERE id = 1").bindparams(
|
||||
d=json.dumps({"name": "LegacyNode", "upstream_api_key": "sk-only-in-db"})
|
||||
)
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
# A reload must not drop the key from the blob...
|
||||
await SettingsService.initialize(session)
|
||||
blob = await _read_settings_blob(session)
|
||||
assert blob["upstream_api_key"] == "sk-only-in-db"
|
||||
# ...and it stays live for the proxy hot path.
|
||||
assert settings.upstream_api_key == "sk-only-in-db"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_existing_blob_secrets_are_stripped_on_initialize(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
@@ -202,9 +228,10 @@ async def test_existing_blob_secrets_are_stripped_on_initialize(
|
||||
blob = await _read_settings_blob(session)
|
||||
assert "admin_password" not in blob
|
||||
assert "nsec" not in blob
|
||||
assert "upstream_api_key" not in blob
|
||||
# Non-secret values survive the migration.
|
||||
# Non-secret values survive the migration, including upstream_api_key,
|
||||
# which is not vaulted yet and so must stay in the blob.
|
||||
assert blob["name"] == "LegacyNode"
|
||||
assert blob["upstream_api_key"] == "sk-legacy"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -212,9 +239,7 @@ async def test_update_does_not_persist_secret_fields(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setenv("NSEC", NSEC_HEX)
|
||||
monkeypatch.setenv("UPSTREAM_API_KEY", "sk-upstream")
|
||||
monkeypatch.setattr(settings, "nsec", "")
|
||||
monkeypatch.setattr(settings, "upstream_api_key", "")
|
||||
|
||||
engine = create_async_engine("sqlite+aiosqlite:///:memory:")
|
||||
async with AsyncSession(engine, expire_on_commit=False) as session:
|
||||
@@ -224,5 +249,4 @@ async def test_update_does_not_persist_secret_fields(
|
||||
blob = await _read_settings_blob(session)
|
||||
assert blob["name"] == "Updated"
|
||||
assert "nsec" not in blob
|
||||
assert "upstream_api_key" not in blob
|
||||
assert "admin_password" not in blob
|
||||
|
||||
Reference in New Issue
Block a user