feat(v2): collision guard — refuse machines whose npub matches an operator account (#32)

Adds `_assert_no_pubkey_collision` to `views_api`, wired into
`api_create_machine` between the wallet-ownership guard and the
`create_machine` CRUD call. Refuses with HTTP 400 + operator-actionable
error message if the supplied `machine_npub` matches any existing
LNbits operator account's `accounts.pubkey`.

## Why this matters

Reproducer 2026-05-30T21:33Z (coord-log archive `2026-05-31-pre-rotation.md`):
Greg's operator account `accounts.pubkey` had been seeded as the same
value as Sintra's `dca_machines.machine_npub` (`522a4538…`) during
manual setup. The collision masked the routing bug for days — lnbits'
nostr-transport `auth.py:resolve_nostr_auth` was routing inbound
kind-21000 RPCs from the ATM directly to Greg's wallet *by coincidence*
of the matching pubkey. When Greg's account migrated to
`RemoteBunkerSigner` and got a fresh pubkey, the coincidence broke +
`auto-account-from-npub` fired for the orphaned ATM npub. A real $20
test cash-out silently landed on a fresh auto-account wallet
(`a94b564f…`); satmachineadmin lost the settlement entirely — no
`dca_settlements` row, no DCA distribution, no commission split.

The proper architectural fix is path B / `aiolabs/satmachineadmin#20`
(S6, in-progress with lnbits — coord-log `2026-05-31T15:25Z`). This
guard is the complementary preventive layer: stops a future operator
from re-entering the broken state by registering a machine whose npub
collides with an existing account.

## What's in this commit

- **`views_api._assert_no_pubkey_collision`** — canonicalises the input
  npub (accepts hex or `npub1…` bech32) via `normalize_public_key`,
  queries `lnbits.core.crud.users.get_account_by_pubkey` (which itself
  lowercases internally), raises HTTPException(400) on hit. Error
  message names the canonical pubkey prefix, explains the
  pubkey-collision dependency that breaks on operator pubkey rotation,
  + points to the `lamassu-next provision-atm` remediation path +
  this issue for context.
- **Wired into `api_create_machine`** after `_assert_wallet_owned_by`
  + before `create_machine`. `api_update_machine` is unaffected
  because `UpdateMachineData` doesn't allow npub changes on existing
  rows.
- **`tests/test_collision_guard.py`** — 7 unit tests covering hex /
  bech32 / uppercase-hex inputs all canonicalise to the same lookup,
  the no-collision case returns silently, error message asserts
  (truncated pubkey + remediation hint). Uses pytest monkeypatch to
  isolate the assertion logic from a live `get_account_by_pubkey` DB
  call — matches the assertion-style pattern of
  `tests/test_nostr_attribution.py`.
- **`CLAUDE.md`** — new "No-collision invariant" subsection under
  Security Considerations: documents the rule + the SQL check
  operators can run on existing installs + the
  `ATM_PRIVATE_KEY`-unset remediation + cross-refs to `#20` and `#32`.

## Regtest SQL check result

Ran the diagnostic SQL against the regtest LNbits + satmachineadmin DBs:

- 1 active `dca_machines.machine_npub`: `522a4538…` (Greg's Sintra)
- 1 collision found: the auto-account orphan `a94b564f…` (username =
  None — auto-account signature) created during yesterday's silent-drop
  failure mode. NOT a legitimate operator account. Greg's actual
  operator account `ac35c9fc…` carries pubkey `197a4cf4…` post-bunker
  migration, no collision there.

The orphan is operational cleanup (sweep + delete), separate from this
code fix. No real-operator collisions remain on the regtest instance.

## Test status

162 passed, 1 pre-existing async-plugin failure unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Padreug 2026-05-31 18:45:56 +02:00
commit 05c1105897
3 changed files with 199 additions and 0 deletions

View file

@ -219,6 +219,38 @@ commission_amount = 266800 - 258835 = 7,965 sats (to commission wallet)
- Input sanitization and type validation
- Audit logging for all administrative actions
### No-collision invariant — operator account pubkey ≠ ATM npub
`dca_machines.machine_npub` and `accounts.pubkey` MUST NEVER hold the
same value across the LNbits instance. Enforced by
`views_api._assert_no_pubkey_collision` at machine-creation time
(rejects with HTTP 400) and by the matching SQL check operators can run
on existing installs:
```sql
SELECT a.id, a.username, a.pubkey, m.id, m.machine_npub
FROM accounts a
JOIN ext_satoshimachine.dca_machines m
ON LOWER(a.pubkey) = LOWER(m.machine_npub);
```
**Why this matters**: when the two values match, lnbits' nostr-transport
`auth.py:resolve_nostr_auth` routes inbound kind-21000 RPCs from the
ATM directly to that operator's wallet *by collision* — it works by
coincidence, breaks silently the moment the operator's pubkey rotates
(then `auto-account-from-npub` fires for the orphaned ATM npub, and the
invoice lands on a fresh auto-account wallet instead). Reproduced on
2026-05-30 against Greg's Sintra (silent cash-out drop). The proper
architectural routing fix is `aiolabs/satmachineadmin#20` (path B /
S6); the collision guard prevents the broken state from being entered
in the first place.
When provisioning a new ATM via `lamassu-next deploy/nixos/provision-atm.sh`,
**leave `ATM_PRIVATE_KEY` unset** so the script generates a fresh ATM
keypair (distinct from any operator's nsec). See
`aiolabs/satmachineadmin#32` for design rationale + the (eventual)
reverse-direction guard on account creation in lnbits proper.
## Development Workflow
### Adding New Features