fix(admin): make create_new_key idempotent — never clobber an existing key (#39) #40

Merged

padreug merged 1 commit from fix-create-new-key-idempotent into dev

2026-06-21 14:01:48 +00:00

padreug commented

2026-06-21 13:51:57 +00:00

Owner

Fixes #39.

The bug

create_new_key.ts unconditionally NDKPrivateKeySigner.generate()s and lets saveEncrypted(...keyName) overwrite config.keys[keyName] on disk — no existing-name guard. Calling it with a name that already exists silently destroys the in-use signing key (the old nsec then lives only in the running process until the next restart, then it's gone — no backup, no export RPC).

This violates the contract spirekeeper's pair_spire documents and relies on (pairing.py: "create_new_key is idempotent — returns the existing key if the name is taken"). A re-pair through the same spire-<id> keyName rotated a machine's spire identity (437d8fd1… → a new 679ac2a8…) and orphaned its tokens / beacon / wallet routing, unrecoverably. Reproduced on the dev stack 2026-06-21.

The fix

Guard at the top of the command — if a key with the name already exists, recover and return it instead of generate-and-overwrite:

no _nsec → decrypt the existing entry with the supplied passphrase, return its npub (the idempotent re-pair path callers expect);
explicit _nsec, an unrecognized/!iv/!data entry, or a passphrase that doesn't decrypt the existing key → throw rather than overwrite. Destroying a key must never be a silent side effect of "create".

The fresh-key path (name not taken) is unchanged.

Verification

Functionally verified on the dev bunker (rebuilt on this branch):

repeat POST …/machines/{id}/pair for an already-paired machine
→ spire_pubkey_hex: 679ac2a8…  (the EXISTING key)
→ config keys['spire-…'] unchanged
VERDICT: IDEMPOTENT ✅

Before the fix the same call minted a new identity and clobbered the on-disk blob.

tsc --noEmit adds no new errors (3 pre-existing in authorize.ts unchanged); npm run build green.

Note on tests

An automated unit test is blocked by the NDK 3.x ESM / ts-node(CJS) harness limitation (the same reason acl/index.ts uses import type) — create_new_key imports NDKPrivateKeySigner at runtime, which the CJS test harness can't load. Flagged in #39 as a follow-up; an ESM-capable harness would let this land with a regression test. Functional end-to-end verification above stands in for now.

🤖 Generated with Claude Code

Fixes #39. ## The bug `create_new_key.ts` unconditionally `NDKPrivateKeySigner.generate()`s and lets `saveEncrypted(...keyName)` overwrite `config.keys[keyName]` on disk — **no existing-name guard**. Calling it with a name that already exists silently destroys the in-use signing key (the old nsec then lives only in the running process until the next restart, then it's gone — no backup, no export RPC). This violates the contract spirekeeper's `pair_spire` documents and relies on (`pairing.py`: *"create_new_key is idempotent — returns the existing key if the name is taken"*). A re-pair through the same `spire-<id>` keyName rotated a machine's spire identity (`437d8fd1…` → a new `679ac2a8…`) and orphaned its tokens / beacon / wallet routing, unrecoverably. Reproduced on the dev stack 2026-06-21. ## The fix Guard at the top of the command — if a key with the name already exists, recover and return it instead of generate-and-overwrite: - **no `_nsec`** → decrypt the existing entry with the supplied passphrase, return its npub (the idempotent re-pair path callers expect); - **explicit `_nsec`**, an unrecognized/`!iv`/`!data` entry, or a passphrase that doesn't decrypt the existing key → **throw** rather than overwrite. Destroying a key must never be a silent side effect of "create". The fresh-key path (name not taken) is unchanged. ## Verification Functionally verified on the dev bunker (rebuilt on this branch): ``` repeat POST …/machines/{id}/pair for an already-paired machine → spire_pubkey_hex: 679ac2a8… (the EXISTING key) → config keys['spire-…'] unchanged VERDICT: IDEMPOTENT ✅ ``` Before the fix the same call minted a new identity and clobbered the on-disk blob. `tsc --noEmit` adds no new errors (3 pre-existing in `authorize.ts` unchanged); `npm run build` green. ## Note on tests An automated unit test is blocked by the NDK 3.x ESM / ts-node(CJS) harness limitation (the same reason `acl/index.ts` uses `import type`) — `create_new_key` imports `NDKPrivateKeySigner` at runtime, which the CJS test harness can't load. Flagged in #39 as a follow-up; an ESM-capable harness would let this land with a regression test. Functional end-to-end verification above stands in for now. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

padreug added 1 commit

2026-06-21 13:51:57 +00:00

fix(admin): make create_new_key idempotent — never clobber an existing key

Docker image / build-and-push-image (push) Has been cancelled

Details

2cf3e55bf4

`create_new_key` unconditionally generated a fresh keypair and let
`saveEncrypted` overwrite `config.keys[keyName]` on disk. So calling it
with a name that already exists SILENTLY DESTROYED the in-use signing
key: the old encrypted nsec was overwritten, surviving only in the
running process's in-memory Backend until the next restart, then gone.

This breaks the contract callers already rely on. spirekeeper's
`pair_spire` re-pairs a machine through the same `spire-<id>` keyName and
documents the assumption verbatim — "create_new_key is idempotent —
returns the existing key if the name is taken" (pairing.py). It wasn't:
a re-pair rotated the machine's spire identity and orphaned everything
bound to the old key (tokens, beacons, wallet routing), unrecoverably.

Guard at the top of the command: if a key with this name already exists,
recover and return it instead of generating + overwriting.
  - no `_nsec`: decrypt the existing entry with the supplied passphrase
    and return its npub (the idempotent re-pair path).
  - explicit `_nsec`, or an unrecognized/!iv/!data entry, or a passphrase
    that doesn't decrypt the existing key: throw rather than overwrite —
    destroying a key must never be a silent side effect of "create".

Functionally verified on the dev bunker: a repeat `/pair` for an
already-paired machine now returns the existing spire pubkey with the
on-disk key entry unchanged, where before it minted a new identity and
clobbered the old blob.

padreug merged commit 4a0a3e7cd4 into dev

2026-06-21 14:01:48 +00:00

padreug deleted branch fix-create-new-key-idempotent

2026-06-21 14:01:48 +00:00

padreug referenced this pull request from a commit

2026-06-21 14:01:49 +00:00

Merge pull request 'fix(admin): make create_new_key idempotent — never clobber an existing key (#39)' (#40) from fix-create-new-key-idempotent into dev

padreug referenced this pull request

2026-06-21 14:13:23 +00:00

create_new_key clobbers an existing key — silent, unrecoverable identity loss on re-pair #39