fix(secrets): make nsec ownership explicit and claim the admin password atomically

Two bootstrap-correctness fixes on the encrypted Secret store:

- Track nsec ownership with an explicit nsec_state (legacy | encrypted |
  cleared) instead of a nsec_managed bool. The bool could not tell "never
  migrated" apart from "intentionally cleared" — both leave encrypted_nsec
  empty — so a cleared identity could be resurrected on a fresh process from a
  stale legacy NSEC (env or old settings blob) and re-derive its npub. Bootstrap
  now branches purely on the state: encrypted decrypts (a missing ciphertext is
  a fail-fast inconsistency, never a silent fall-through to legacy), cleared
  actively empties the live nsec and npub, and legacy imports the plaintext
  once.
- Claim a generated admin password atomically. When no password exists, the
  generated one is written via a conditional UPDATE (WHERE admin_password_hash
  IS NULL) and only the worker that wins the update (rowcount 1) prints it. A
  racing worker on a shared database adopts the winner's hash and stays silent,
  so the operator never sees a second password that was never stored.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

migrations/versions/c6f8d2e4a1b3_add_secrets_table.py

@@ -6,9 +6,10 @@ 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. ``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.
+where the live ROUTSTR_SECRET_KEY is available. ``nsec_state`` records the vault's
+ownership of the nsec (legacy | encrypted | cleared), so a cleared identity is
+never resurrected from a stale legacy ``NSEC`` env var / settings blob on the next
+boot.
 """
 
 import sqlalchemy as sa
@@ -36,10 +37,10 @@ def upgrade() -> None:
             nullable=True,
         ),
         sa.Column(
-            "nsec_managed",
-            sa.Boolean(),
+            "nsec_state",
+            sqlmodel.sql.sqltypes.AutoString(),
             nullable=False,
-            server_default=sa.false(),
+            server_default="legacy",
         ),
         sa.Column("updated_at", sa.Integer(), nullable=True),
         sa.PrimaryKeyConstraint("id"),

routstr/core/db.py

@@ -6,6 +6,7 @@ import sqlite3
 import time
 import uuid
 from contextlib import asynccontextmanager
+from enum import Enum
 from typing import AsyncGenerator
 
 from alembic import command
@@ -443,6 +444,26 @@ class RoutstrFee(SQLModel, table=True):  # type: ignore
     payout_started_at: int | None = Field(default=None)
 
 
+class NsecState(str, Enum):
+    """Ownership state of the node's nsec — an explicit 3-state machine.
+
+    The single ``encrypted_nsec`` column cannot distinguish "never migrated" from
+    "intentionally cleared" (both leave it empty), which let a cleared identity be
+    resurrected from a stale legacy ``NSEC``. This names the three states so the
+    bootstrap branches on ownership rather than inferring it:
+
+    * ``legacy`` — the vault has not taken ownership; a plaintext ``NSEC`` (env or
+      old settings blob) may still exist and should be migrated in once.
+    * ``encrypted`` — the vault owns a ciphertext; decrypt it, never re-read env.
+    * ``cleared`` — the vault owns it but the operator emptied it; stay empty,
+      never re-import from a stale legacy copy.
+    """
+
+    legacy = "legacy"
+    encrypted = "encrypted"
+    cleared = "cleared"
+
+
 class Secret(SQLModel, table=True):  # type: ignore
     """Node-level secrets, stored encrypted/hashed at rest (singleton, id=1).
 
@@ -455,11 +476,7 @@ 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)
+    nsec_state: NsecState = Field(default=NsecState.legacy)
     updated_at: int | None = Field(default=None)
 
 
@@ -536,15 +553,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). 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.
+    no events). Either way the vault now owns the nsec, so the state moves off
+    ``legacy``: a cleared identity (``cleared``) 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.nsec_state = NsecState.encrypted if nsec else NsecState.cleared
     secret.updated_at = int(time.time())
     session.add(secret)
     await session.commit()

routstr/core/settings.py

@@ -467,9 +467,10 @@ async def bootstrap_secrets(db_session: AsyncSession) -> None:
         hash it, and log it once with the /admin URL.
     """
     from cryptography.fernet import InvalidToken
+    from sqlmodel import col, update
 
     from . import vault
-    from .db import get_secret
+    from .db import NsecState, Secret, get_secret
 
     raw_blob = await _read_raw_settings_blob(db_session)
     secret = await get_secret(db_session)
@@ -482,23 +483,52 @@ async def bootstrap_secrets(db_session: AsyncSession) -> None:
         )
         if legacy_password:
             secret.admin_password_hash = vault.hash_password(legacy_password)
+            changed = True
         else:
             generated = secrets.token_urlsafe(24)
-            secret.admin_password_hash = vault.hash_password(generated)
-            admin_url = (settings.http_url or "http://localhost:8000").rstrip("/")
-            # Print to stdout rather than the logger: the operator must see this
-            # once (e.g. `docker compose logs`), but it must not be persisted
-            # into the on-disk log files the logger also writes to.
-            print(
-                "No admin password set; generated a temporary one (shown only "
-                f"now): {generated}\nLog in at {admin_url}/admin and change it "
-                "from the dashboard settings.",
-                flush=True,
+            # Claim the empty slot atomically: only the worker whose UPDATE flips
+            # NULL -> hash owns the generated password and announces it. On a
+            # shared DB a racing worker gets rowcount 0, so it neither clobbers
+            # the winner's hash (which the operator may already be using) nor
+            # prints a second password that would never work.
+            claim_stmt = (
+                update(Secret)
+                .where(col(Secret.id) == 1)
+                .where(col(Secret.admin_password_hash).is_(None))
+                .values(
+                    admin_password_hash=vault.hash_password(generated),
+                    updated_at=int(time.time()),
+                )
             )
-        changed = True
+            result = await db_session.exec(claim_stmt)  # type: ignore[call-overload]
+            await db_session.commit()
+            await db_session.refresh(secret)
+            if result.rowcount == 1:
+                admin_url = (settings.http_url or "http://localhost:8000").rstrip("/")
+                # Print to stdout rather than the logger: the operator must see
+                # this once (e.g. `docker compose logs`), but it must not be
+                # persisted into the on-disk log files the logger also writes to.
+                print(
+                    "No admin password set; generated a temporary one (shown "
+                    f"only now): {generated}\nLog in at {admin_url}/admin and "
+                    "change it from the dashboard settings.",
+                    flush=True,
+                )
 
-    # Nostr nsec — reversible Fernet encryption.
-    if secret.encrypted_nsec is not None:
+    # Nostr nsec — reversible Fernet encryption. ``nsec_state`` is the single
+    # source of truth for ownership, so "intentionally cleared" is never
+    # conflated with "never migrated" (the bug the old bool could not encode).
+    if secret.nsec_state == NsecState.encrypted:
+        # The vault owns the identity: decrypt the ciphertext, never re-read
+        # env/blob. A missing ciphertext here means the row is inconsistent (a
+        # failed write or manual edit); fail fast rather than silently dropping
+        # the identity and falling back to a stale legacy copy.
+        if secret.encrypted_nsec is None:
+            raise RuntimeError(
+                "nsec_state is 'encrypted' but no ciphertext is stored; the "
+                "secrets row is inconsistent. Refusing to boot rather than "
+                "silently resurrecting a stale legacy NSEC."
+            )
         try:
             settings.nsec = vault.decrypt(secret.encrypted_nsec)
         except InvalidToken as exc:
@@ -507,21 +537,23 @@ 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
-    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.
+    elif secret.nsec_state == NsecState.cleared:
+        # The operator emptied the identity via the admin API. A fresh process
+        # has already reloaded a stale ``NSEC`` from env/blob into the live
+        # settings (and may have derived its npub); actively clear both so the
+        # cleared store wins rather than silently resurrecting the old identity.
+        settings.nsec = ""
+        settings.npub = ""
+    else:  # NsecState.legacy — the vault has not taken ownership yet.
+        # Import any legacy plaintext (env, or the old settings blob) exactly
+        # once. Encryption at rest is mandatory, but a missing key is
+        # provisioned, not fatal: vault.encrypt generates and persists a master
+        # key (with a loud one-time operator notice) when none was supplied, so
+        # an upgrading node keeps running. The nsec is never stored in plaintext.
         legacy_nsec = _legacy_plaintext(raw_blob, "NSEC", "nsec")
         if legacy_nsec:
-            # The node has a Nostr identity to protect. Encryption at rest is
-            # mandatory, but a missing key is provisioned, not fatal:
-            # vault.encrypt generates and persists a master key (with a loud
-            # one-time operator notice) when none was supplied, so an upgrading
-            # node keeps running. The nsec is never persisted in plaintext.
             secret.encrypted_nsec = vault.encrypt(legacy_nsec)
-            secret.nsec_managed = True
+            secret.nsec_state = NsecState.encrypted
             settings.nsec = legacy_nsec
             changed = True
 

tests/integration/test_secret_bootstrap.py

@@ -18,7 +18,7 @@ from sqlmodel import text
 from sqlmodel.ext.asyncio.session import AsyncSession
 
 from routstr.core import vault
-from routstr.core.db import get_secret, set_nsec
+from routstr.core.db import NsecState, get_secret, set_nsec
 from routstr.core.settings import (
     SettingsService,
     bootstrap_secrets,
@@ -111,6 +111,57 @@ async def test_hashes_legacy_admin_password_from_blob(
     assert vault.verify_password("blobpw", secret.admin_password_hash or "") is True
 
 
+@pytest.mark.asyncio
+async def test_admin_password_race_adopts_winner_without_clobber(
+    clean_secret_env: None,
+    integration_engine: Any,
+    integration_session: AsyncSession,
+    monkeypatch: pytest.MonkeyPatch,
+    capsys: pytest.CaptureFixture[str],
+) -> None:
+    # Two workers boot against one shared DB and both read a null admin password.
+    # The first to commit "wins" and shows the operator its generated password. A
+    # worker that read null but lost the race must NOT overwrite the winner's hash
+    # (which the operator may already be logging in with) and must NOT print a
+    # second password that will never work.
+    #
+    # The race window is forced deterministically: a hook fires inside bootstrap's
+    # generate branch (so it only runs once this worker has committed to
+    # generating) and commits the winner's password on a separate connection
+    # before this worker writes its own.
+    import sqlite3
+
+    from routstr.core import settings as settings_mod
+
+    db_file = integration_engine.url.database
+    winner_hash = vault.hash_password("winner-password-123")
+    real_token = settings_mod.secrets.token_urlsafe
+
+    def commit_winner_then_generate(nbytes: int) -> str:
+        conn = sqlite3.connect(db_file)
+        conn.execute(
+            "UPDATE secrets SET admin_password_hash = ? WHERE id = 1", (winner_hash,)
+        )
+        conn.commit()
+        conn.close()
+        return real_token(nbytes)
+
+    monkeypatch.setattr(
+        settings_mod.secrets, "token_urlsafe", commit_winner_then_generate
+    )
+
+    await get_secret(integration_session)  # row exists, password still null
+    capsys.readouterr()  # drop anything emitted before the race resolves
+    await bootstrap_secrets(integration_session)
+
+    secret = await get_secret(integration_session)
+    assert secret.admin_password_hash is not None
+    # The winner's password survives and still verifies — no clobber.
+    assert vault.verify_password("winner-password-123", secret.admin_password_hash)
+    # The losing worker stayed silent — no second generated password was leaked.
+    assert "generated a temporary" not in capsys.readouterr().out
+
+
 # --- nsec ------------------------------------------------------------------
 
 
@@ -137,6 +188,7 @@ async def test_decrypts_existing_nsec_column(
 ) -> None:
     secret = await get_secret(integration_session)
     secret.encrypted_nsec = vault.encrypt(NSEC_HEX)
+    secret.nsec_state = NsecState.encrypted
     integration_session.add(secret)
     await integration_session.commit()
     stored = secret.encrypted_nsec
@@ -159,6 +211,7 @@ async def test_fail_fast_when_nsec_encrypted_with_different_key(
     monkeypatch.setenv("ROUTSTR_SECRET_KEY", TEST_SECRET_KEY_ALT)
     secret = await get_secret(integration_session)
     secret.encrypted_nsec = vault.encrypt(NSEC_HEX)
+    secret.nsec_state = NsecState.encrypted
     integration_session.add(secret)
     await integration_session.commit()
 
@@ -197,7 +250,7 @@ async def test_legacy_nsec_without_secret_key_generates_and_encrypts(
     assert secret.encrypted_nsec is not None
     assert vault.is_encrypted(secret.encrypted_nsec) is True
     assert vault.decrypt(secret.encrypted_nsec) == NSEC_HEX
-    assert secret.nsec_managed is True
+    assert secret.nsec_state == NsecState.encrypted
     # ...the node holds the live identity (npub derived from it)...
     assert settings.nsec == NSEC_HEX
     assert settings.npub == derive_npub_from_nsec(NSEC_HEX)
@@ -253,6 +306,7 @@ async def test_initialize_does_not_clobber_store_only_nsec(
     await _create_settings_blob(integration_session, {"name": "LegacyNode"})
     secret = await get_secret(integration_session)
     secret.encrypted_nsec = vault.encrypt(NSEC_HEX)
+    secret.nsec_state = NsecState.encrypted
     integration_session.add(secret)
     await integration_session.commit()
 
@@ -325,23 +379,29 @@ async def test_cleared_nsec_stays_cleared_across_reboot(
     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
+    # admin API. The old NSEC is still in env. On the NEXT PROCESS 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).
+    # Clear via the admin path (store empty, vault owns it).
     await set_nsec(integration_session, "")
-    monkeypatch.setattr(settings, "nsec", "")
 
-    # Reboot with the stale NSEC still present in env.
+    # Simulate a fresh process rather than pre-clearing the live singleton: the
+    # pydantic settings global reloads the (still-stale) NSEC from env and derives
+    # its npub, which is exactly the in-memory state a new boot starts from before
+    # bootstrap runs. The cleared store must win over this stale live value.
+    monkeypatch.setattr(settings, "nsec", NSEC_HEX)
+    monkeypatch.setattr(settings, "npub", derive_npub_from_nsec(NSEC_HEX))
+
     await bootstrap_secrets(integration_session)
 
     reloaded = await get_secret(integration_session)
-    assert reloaded.nsec_managed is True
+    assert reloaded.nsec_state == NsecState.cleared
     assert reloaded.encrypted_nsec is None  # not re-imported
     assert settings.nsec == ""  # stays cleared
+    assert settings.npub == ""  # and no derived public identity survives
 
 
 @pytest.mark.asyncio
@@ -360,6 +420,7 @@ async def test_initialize_keeps_npub_matching_store_only_nsec(
     await _create_settings_blob(integration_session, {"name": "LegacyNode"})
     secret = await get_secret(integration_session)
     secret.encrypted_nsec = vault.encrypt(NSEC_HEX)
+    secret.nsec_state = NsecState.encrypted
     integration_session.add(secret)
     await integration_session.commit()