test: end-to-end coverage for encrypted secret storage

[jeroenubbink]

What

End-to-end coverage for the node's encrypted secret storage
(Routstr/routstr-core#570). These exercise whole-system behaviours that the
node's own unit/integration tests can't reach: a real container boot, secrets
supplied through the real environment, the real on-disk SQLite file, and the
live HTTP API.

Two scenarios:

secrets_at_rest — secrets are never stored in plaintext

Against a running node (3 tests):

• GET /admin/api/settings does not carry an admin_password field.
• PATCH /admin/api/nsec accepts an nsec and returns the derived npub.
• After forcing a settings write, the on-disk SQLite (including the WAL
  sidecar) contains no plaintext admin password and no bech32 nsec — guarded
  against a vacuous pass by first asserting a probe value did land on disk.

secret_lifecycle — boot-time behaviour

Boots throwaway nodes with tailored environments to cover what only happens at
startup (4 tests):

• refuses to boot (and prints the key-generation command) when a stored nsec
  needs ROUTSTR_SECRET_KEY but it is absent;
• on first run with no admin password, generates one, logs it once, and that
  password logs in;
• a key change between boots bricks the encrypted nsec and the node fails fast;
• a stored nsec survives a later boot with no nsec in the environment, and the
  npub stays stable.

How it works

• A small ephemeral-boot harness (tests/integration/node_boot.py) drives the
  already-built node image directly with docker run on the compose network,
  using throwaway volumes, so each test gets a node with its own environment
  without disturbing the standing stack.
• Both scenarios are marked destructive and require services to be up
  (services_required: true), so they don't pass vacuously when the stack is
  skipped.
• A fixed ROUTSTR_SECRET_KEY is added to both compose nodes (required by the
  node under test, ignored by older nodes).

Verified red→green by toggling the corresponding node implementation in the
vendored checkout: red on the pre-change node, green after.

Independent of the in-flight input-fee scenario (#4).

🤖 Generated with Claude Code

[9qeklajc]

Thanks for this — the scenario design is solid (probe-guard against vacuous passes, red→green verified against the vendored node, ephemeral boots that don't disturb the standing stack). Findings below were cross-checked against the actual node implementation in Routstr/routstr-core#570.

Worth fixing now

1. The at-rest grep misses the secret it should most care about (test_secrets_at_rest.py:169-178)

Two independent ways the "no plaintext secrets on disk" check can pass vacuously:

• Rotated nsec is never grepped. test_nsec_rotation_endpoint_derives_npub runs first (definition order) and replaces the stored nsec with ROTATE_NSEC_HEX ("1"*64). The DB check then only asserts the original bech32 NODE_NSEC is absent. The node stores the nsec verbatim as supplied (no bech32↔hex normalization — the hex/bech32 parsing in #570 is validation-only), so after rotation the node's live secret is literally the string "1"*64, and a regression persisting it in plaintext still passes GREEN. One-line fix: also assert ROTATE_NSEC_HEX.encode() not in db_bytes.
• .env divergence. NODE_NSEC falls back to the compose default from the pytest process env, but the documented operator path is setting NODE_A_NSEC in .env, which compose reads and pytest never loads. In that setup the node boots with the real nsec while the test greps the default. Worth either loading .env in the test/conftest or documenting that these tests assume the compose defaults.

2. Fixed host port 8077 can wedge the suite (node_boot.py:37)

serving_node always binds host port 8077, containers are started without --name or a label, and cleanup lives only in finally blocks. A run killed hard (SIGKILL/CI timeout) leaves an anonymous container serving on 8077 indefinitely; every subsequent run then fails docker run → pytest.fail under SERVICES_REQUIRED=1 until someone cleans up by hand. Suggested: use an ephemeral host port (read it back via docker inspect/docker port), or at least label the containers and remove stale ones at module setup.

Worth considering

• Rotation permanently mutates the standing node-a (test_secrets_at_rest.py:106). Fine under the default orchestrated run (down -v teardown), but with KEEP_UP=1 or the documented make up reuse flow, node-a keeps announcing under the rotated npub for later discovery scenarios, and a subsequent restart with env NSEC ≠ stored nsec can brick the stack. Restoring the original nsec at the end of the test would make the scenario safe under stack reuse.
• Ephemeral nodes announce on the shared relay (node_boot.py:93). Lifecycle nodes join ws://relay:8080 with HTTP_URL=http://localhost:8000; the node announces unconditionally whenever an nsec is present (no disable flag in #570), and the relay's named volume keeps the dead "LifecycleNode" announcement after the container is gone, which discovery scenarios (e.g. routstrd_cheapest) on a reused stack may then see. Pointing ephemeral nodes at no relay (or a throwaway one) avoids the pollution.
• admin_login() duplicates targets.admin_token() (node_boot.py:242) — same endpoint, payload, timeout, token extraction and exception handling. Extracting a shared _mint_admin_token(base_url, password) helper in targets.py keeps the login contract in one place.
• Document the new compose vars. NODE_A_ROUTSTR_SECRET_KEY / NODE_B_ROUTSTR_SECRET_KEY are added in compose.yml but absent from .env.example and the README config section, so operators have no cue that a second mandatory secret var now exists (with a publicly committed default).

Checked and found fine

• Key-change brick test keeping NSEC in the boot-2 env (test_secret_lifecycle.py:127): I initially read this as over-constraining (an implementation could recover by re-encrypting the env nsec under the new key), but #570 makes fail-fast the deliberate contract — bootstrap_secrets() never consults the env nsec when a stored ciphertext exists, and the docstring documents that a wrong ROUTSTR_SECRET_KEY "surfaces as a clear fail-fast". Keeping NSEC in the env actually strengthens the coverage: it proves the node refuses to silently re-migrate even when it could.
• The pinned log strings ("Stored nsec cannot be decrypted…", "Fernet.generate_key", the "shown only now" password line) all exist verbatim in the node.
• The 600s secret_lifecycle budget has ~30% headroom over the all-worst-case wall clock, and boot_until_settled's full-window wait only costs time on runs that have already failed.

Out of scope but observed

Under TARGET_PROFILE=remote, destructive tests are collected-but-skipped, so pytest exits 0 and the orchestrator reports green with zero verification (the rc == 5 "no tests collected" guard doesn't fire). That property predates this PR — three existing destructive scenarios behave the same — but it's worth a harness-level fix (an all-skipped guard) in a separate issue, since these two scenarios inherit it.

[jeroenubbink]

Thanks for the thorough pass — every finding landed. Pushed a three-commit response (186ecef, c1ce536, 1939083).

Worth fixing now

1. At-rest grep vacuous passes — fixed at the root rather than with a one-liner. Both failure modes shared one cause: the test ran against the shared standing node-a with definition-order coupling. Moved the whole file onto the ephemeral node_boot harness (the same one the lifecycle scenario uses) — each test now boots its own throwaway node on a fresh volume, seeded with a known admin password and nsec through container env. The DB-inspection test seeds → forces a settings persist → rotates the identity → asserts the admin password, the seeded nsec, and the freshly rotated ROTATE_NSEC_HEX are all absent, on one isolated node. Seeding through the front door also dissolves the .env-divergence point: the test controls the exact plaintext, so there's no compose-vs-pytest seed mismatch to document around.

2. Fixed port 8077 wedge — fixed. serving_node now publishes to an ephemeral host port (-p 0:8000, read back via docker port), every ephemeral container carries a routstr-lifecycle-e2e label, and stale ones are reaped once at module setup. A hard-killed run can no longer hold a port or leak an anonymous container into the next run.

Worth considering

• Rotation mutating standing node-a — now moot: the at-rest tests no longer touch node-a at all (throwaway volumes per test), so there's nothing to restore and no KEEP_UP=1 reuse hazard.
• Relay pollution — I think this one doesn't fire: ephemeral nodes keep HTTP_URL=http://localhost:8000, which the node treats as "no real endpoint" and skips the listing publish entirely (announce_provider logs "No valid endpoints… Skipping listing publish"), so a lifecycle node never announces onto the shared relay even with an nsec present. I left RELAYS pointed at the in-stack relay deliberately — clearing it is worse, since an empty list falls back to public relays. Documented the reasoning in base_node_env. Let me know if you're seeing an announce slip through despite the sentinel.
• admin_login duplication — extracted targets.mint_admin_token(base_url, password) as the single login home; admin_token and the boot harness both route through it and node_boot.admin_login is gone. (Also folded the Bearer header into targets.bearer_headers.)
• Document the compose vars — added NODE_A/B_ROUTSTR_SECRET_KEY to .env.example and the README config section.

Out of scope — agreed on the TARGET_PROFILE=remote all-skipped-exits-0 gap; it predates this PR and affects the existing destructive scenarios too, so I'll file it as a separate harness-level issue (an all-skipped guard) rather than fold it in here.