From 0415806c5ac8a074885382d74f732e3a598f2c00 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 7 Jul 2026 16:17:30 +0200 Subject: [PATCH] fix(config): keep the vault the sole owner of node secrets Two ways a stale legacy NSEC could override or resurrect an nsec the vault already owns (issue #553): - initialize() re-applied env/blob values onto live settings after bootstrap had decrypted the authoritative nsec, so a stale NSEC left in .env would clobber it on restart (e.g. after rotating the key in the admin UI). _apply_to_live_settings now never re-applies secret fields; bootstrap_secrets is their only writer. - An empty encrypted_nsec could not distinguish "never migrated" from "intentionally cleared", so clearing the identity via the admin API and restarting re-imported the old NSEC from env/blob. Record vault ownership in a new secrets.nsec_managed column (set on legacy import and on every set_nsec write); bootstrap skips the legacy import once the vault owns the nsec, so a cleared identity stays cleared. Co-Authored-By: Claude Opus 4.8 --- .../c6f8d2e4a1b3_add_secrets_table.py | 10 +++- routstr/core/db.py | 10 +++- routstr/core/settings.py | 22 +++++--- tests/integration/test_secret_bootstrap.py | 53 ++++++++++++++++++- tests/unit/test_settings.py | 7 +-- 5 files changed, 88 insertions(+), 14 deletions(-) diff --git a/migrations/versions/c6f8d2e4a1b3_add_secrets_table.py b/migrations/versions/c6f8d2e4a1b3_add_secrets_table.py index cd41d89c..77a5a8d9 100644 --- a/migrations/versions/c6f8d2e4a1b3_add_secrets_table.py +++ b/migrations/versions/c6f8d2e4a1b3_add_secrets_table.py @@ -6,7 +6,9 @@ Create Date: 2026-07-07 00:00:00.000000 Creates the node-level singleton secret store (issue #553). Schema only; moving any legacy plaintext into the encrypted/hashed columns happens at bootstrap, -where the live ROUTSTR_SECRET_KEY is available. +where the live ROUTSTR_SECRET_KEY is available. ``nsec_managed`` records that the +vault has taken ownership of the nsec, so a cleared identity is never resurrected +from a stale legacy ``NSEC`` env var / settings blob on the next boot. """ import sqlalchemy as sa @@ -33,6 +35,12 @@ def upgrade() -> None: sqlmodel.sql.sqltypes.AutoString(), nullable=True, ), + sa.Column( + "nsec_managed", + sa.Boolean(), + nullable=False, + server_default=sa.false(), + ), sa.Column("updated_at", sa.Integer(), nullable=True), sa.PrimaryKeyConstraint("id"), ) diff --git a/routstr/core/db.py b/routstr/core/db.py index 827cc24f..5fe29858 100644 --- a/routstr/core/db.py +++ b/routstr/core/db.py @@ -455,6 +455,11 @@ class Secret(SQLModel, table=True): # type: ignore id: int = Field(default=1, primary_key=True) admin_password_hash: str | None = Field(default=None) encrypted_nsec: str | None = Field(default=None) + # True once the vault owns the nsec (imported from legacy plaintext, or set + # via the admin API). A cleared nsec then stays cleared: bootstrap must not + # resurrect it from a stale legacy ``NSEC`` env var / settings blob, which an + # empty ``encrypted_nsec`` alone cannot distinguish from "never migrated". + nsec_managed: bool = Field(default=False) updated_at: int | None = Field(default=None) @@ -531,12 +536,15 @@ async def set_nsec(session: AsyncSession, nsec: str) -> None: """Store the node's nsec, Fernet-encrypted, on the Secret singleton. An empty string clears it (the node then holds no Nostr identity and signs - no events). + no events). Either way the vault now owns the nsec, so ``nsec_managed`` is + set: a cleared identity must not be resurrected from a stale legacy ``NSEC`` + on the next boot. """ from .vault import encrypt secret = await get_secret(session) secret.encrypted_nsec = encrypt(nsec) if nsec else None + secret.nsec_managed = True secret.updated_at = int(time.time()) session.add(secret) await session.commit() diff --git a/routstr/core/settings.py b/routstr/core/settings.py index 5b9793ee..b3165cc2 100644 --- a/routstr/core/settings.py +++ b/routstr/core/settings.py @@ -153,15 +153,15 @@ def _strip_secret_fields(data: dict[str, Any]) -> dict[str, Any]: def _apply_to_live_settings(data: dict[str, Any]) -> None: """Apply ``data`` onto the live ``settings`` for all in-process importers. - Secrets are owned by ``bootstrap_secrets`` (which decrypts the nsec into - memory before this runs) — they are never persisted to the blob, so ``data`` - re-derived from the secret-free blob carries empty secret values. Skip those - empty overwrites so a live secret is never clobbered; a non-empty value - (legacy env, or a not-yet-stripped blob mid-migration) is still applied. + Secrets are owned exclusively by ``bootstrap_secrets``, which runs first and + has already decrypted the authoritative nsec into memory (importing any + legacy plaintext on the way). Never re-apply secret fields from env/blob + here: a non-empty but stale ``NSEC`` env var would otherwise override an nsec + the vault has taken ownership of (e.g. after the operator rotates it in the + UI), and an empty one would wipe the live value. Skip them entirely. """ - live = settings.dict() for k, v in data.items(): - if k in SECRET_FIELDS and not v and live.get(k): + if k in SECRET_FIELDS: continue setattr(settings, k, v) @@ -504,7 +504,12 @@ async def bootstrap_secrets(db_session: AsyncSession) -> None: "ROUTSTR_SECRET_KEY. The key changed, or this database came from " "another node. Restore the original ROUTSTR_SECRET_KEY to recover." ) from exc - else: + elif not secret.nsec_managed: + # The vault has not taken ownership yet: import any legacy plaintext + # (env, or the old settings blob). Once managed, an empty encrypted_nsec + # means the identity was intentionally cleared via the admin API, so this + # branch is skipped and the nsec stays empty rather than being resurrected + # from a stale legacy copy. legacy_nsec = _legacy_plaintext(raw_blob, "NSEC", "nsec") if legacy_nsec: # The node has a Nostr identity to protect. Encryption at rest is @@ -519,6 +524,7 @@ async def bootstrap_secrets(db_session: AsyncSession) -> None: 'print(Fernet.generate_key().decode())"' ) secret.encrypted_nsec = vault.encrypt(legacy_nsec) + secret.nsec_managed = True settings.nsec = legacy_nsec changed = True diff --git a/tests/integration/test_secret_bootstrap.py b/tests/integration/test_secret_bootstrap.py index 4a8a51db..077a5f74 100644 --- a/tests/integration/test_secret_bootstrap.py +++ b/tests/integration/test_secret_bootstrap.py @@ -17,7 +17,7 @@ from sqlmodel import text from sqlmodel.ext.asyncio.session import AsyncSession from routstr.core import vault -from routstr.core.db import get_secret +from routstr.core.db import get_secret, set_nsec from routstr.core.settings import ( SettingsService, bootstrap_secrets, @@ -30,6 +30,9 @@ TEST_SECRET_KEY = "l_Tkp-7xmjcQ-IFhr6qhILrU8HPRbEmYMrfSbo_5srU=" TEST_SECRET_KEY_ALT = "_Teyrky_iToeDK51Tj1FsI9MJ340_cqKGmeher-a7MQ=" NSEC_HEX = "1" * 64 +# A different key, standing in for a stale value left behind in env/blob after +# the vault has taken ownership of the real one. +STALE_NSEC_HEX = "2" * 64 @pytest.fixture @@ -243,6 +246,54 @@ async def test_initialize_does_not_clobber_store_only_nsec( assert "nsec" not in json.loads(row.first()[0]) +@pytest.mark.asyncio +async def test_stale_env_nsec_does_not_override_vault_nsec( + clean_secret_env: None, + integration_session: AsyncSession, + monkeypatch: pytest.MonkeyPatch, +) -> None: + # The vault owns the nsec, but a stale NSEC (e.g. the operator rotated the + # key in the UI yet left the old value in .env) is still in the environment. + # bootstrap decrypts the store value; initialize must NOT let the stale env + # value clobber it, or a restart silently reverts to the old identity. + await set_nsec(integration_session, NSEC_HEX) + + monkeypatch.setenv("NSEC", STALE_NSEC_HEX) + await _create_settings_blob(integration_session, {"name": "LegacyNode"}) + + await bootstrap_secrets(integration_session) + await SettingsService.initialize(integration_session) + + # The vault value wins; the stale env value is ignored. + assert settings.nsec == NSEC_HEX + + +@pytest.mark.asyncio +async def test_cleared_nsec_stays_cleared_across_reboot( + clean_secret_env: None, + integration_session: AsyncSession, + monkeypatch: pytest.MonkeyPatch, +) -> None: + # An identity was imported from env, then the operator cleared it via the + # admin API. The old NSEC is still in env. On the next boot the cleared + # identity must stay cleared, not get resurrected from the stale env value. + monkeypatch.setenv("NSEC", NSEC_HEX) + await bootstrap_secrets(integration_session) + assert settings.nsec == NSEC_HEX + + # Clear via the admin path (mirrors the endpoint: store empty, live empty). + await set_nsec(integration_session, "") + monkeypatch.setattr(settings, "nsec", "") + + # Reboot with the stale NSEC still present in env. + await bootstrap_secrets(integration_session) + + reloaded = await get_secret(integration_session) + assert reloaded.nsec_managed is True + assert reloaded.encrypted_nsec is None # not re-imported + assert settings.nsec == "" # stays cleared + + @pytest.mark.asyncio async def test_initialize_keeps_npub_matching_store_only_nsec( clean_secret_env: None, integration_session: AsyncSession diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index 6d458470..a553b6d4 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -148,9 +148,10 @@ def test_settings_model_drops_admin_password_field() -> None: async def test_secret_fields_kept_in_memory_but_not_persisted( monkeypatch: pytest.MonkeyPatch, ) -> None: - monkeypatch.setenv("NSEC", NSEC_HEX) - # Reset the live globals so monkeypatch reverts them after the test. - monkeypatch.setattr(settings, "nsec", "") + # bootstrap_secrets (which runs first at boot) owns the nsec and has already + # decrypted it into memory; simulate that live value. initialize must keep it + # in memory for runtime consumers yet never write it to the settings blob. + monkeypatch.setattr(settings, "nsec", NSEC_HEX) engine = create_async_engine("sqlite+aiosqlite:///:memory:") async with AsyncSession(engine, expire_on_commit=False) as session: