diff --git a/routstr/core/settings.py b/routstr/core/settings.py index 8b779e4..b8eacdd 100644 --- a/routstr/core/settings.py +++ b/routstr/core/settings.py @@ -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 diff --git a/tests/integration/test_admin_settings_endpoint.py b/tests/integration/test_admin_settings_endpoint.py index c97a9a9..15d0515 100644 --- a/tests/integration/test_admin_settings_endpoint.py +++ b/tests/integration/test_admin_settings_endpoint.py @@ -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 diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index ffe6352..6d45847 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -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