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

Merged

padreug merged 1 commit from feat/collision-detection into v2-bitspire 2026-05-31 18:10:31 +00:00

Conversation 1 Commits 1 Files changed 3 +199

padreug commented 2026-05-31 16:46:59 +00:00

Owner

Copy link

Summary

Closes aiolabs/satmachineadmin#32. Adds _assert_no_pubkey_collision to views_api, wired into api_create_machine — refuses with HTTP 400 if the supplied machine_npub matches any existing LNbits operator account's accounts.pubkey. Prevents the failure mode reproduced 2026-05-30 on Greg's Sintra (coord-log archive 2026-05-31-pre-rotation.md 21:33Z): operator-account pubkey collision with the ATM npub silently routed cash-out invoices to the operator's wallet by coincidence, then broke silently the moment the operator's pubkey rotated.

Complements the architectural fix in aiolabs/satmachineadmin#20 (path B, in-progress with lnbits side per coord-log 2026-05-31T15:25Z). Two layers of defence: this guard prevents the dependency from being entered; #20's routing layer fixes the architectural shape.

What's in this PR

File	Change
views_api.py	_assert_no_pubkey_collision helper + wire into api_create_machine; new imports get_account_by_pubkey, normalize_public_key
tests/test_collision_guard.py	7 unit tests covering hex / bech32 / uppercase-hex input canonicalisation, positive + negative cases, error-message content. Uses pytest monkeypatch to isolate 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 operator-runnable SQL check, the ATM_PRIVATE_KEY-unset remediation path, + cross-refs to #20 and #32

Regtest SQL check result

Ran bitspire's suggested diagnostic SQL against the regtest LNbits + satmachineadmin DBs (per #32 AC item 1):

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);

Active machine_npub	Collision found
522a4538… (Greg's Sintra)	Auto-account orphan a94b564f… (username=None — auto-account signature, from 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 on the regtest instance.

Acceptance criteria mapping (#32)

• One-time SQL check run on the regtest instance + result documented (above; also posted as comment on #32).
• api_create_machine rejects with 400 + clear error message when machine_npub matches any existing accounts.pubkey. Tested with hex, bech32, and uppercase-hex inputs.
• Documentation note in CLAUDE.md explaining the no-collision invariant + pointing at #32 for context.
• Follow-up filed upstream against aiolabs/lnbits for the reverse-direction account-creation guard. Not in this PR — will file separately after the path-B handoff with lnbits is settled (coord-log 2026-05-31T15:25Z).

Test plan

• Unit tests: 7 new + 155 existing pass cleanly (docker compose exec lnbits /app/.venv/bin/python -m pytest tests/ → 162 passed)
• Pre-existing async-plugin failure (test_init.py::test_router) unchanged — not introduced by this PR
• Manual smoke: try registering a machine through the UI with a colliding npub → expect 400 with the operator-actionable error message + #32 link

Out of scope

• The architectural routing fix (path B / #20) — independent track with lnbits, partially scoped via coord-log thread
• Sweeping the auto-account orphan wallet's balance back to an operator wallet — operational, not a code change
• Reverse-direction guard at LNbits account-creation time — needs to be filed against aiolabs/lnbits as a follow-up

🤖 Generated with Claude Code

## Summary Closes `aiolabs/satmachineadmin#32`. Adds `_assert_no_pubkey_collision` to `views_api`, wired into `api_create_machine` — refuses with HTTP 400 if the supplied `machine_npub` matches any existing LNbits operator account's `accounts.pubkey`. Prevents the failure mode reproduced 2026-05-30 on Greg's Sintra (coord-log archive `2026-05-31-pre-rotation.md` 21:33Z): operator-account pubkey collision with the ATM npub silently routed cash-out invoices to the operator's wallet *by coincidence*, then broke silently the moment the operator's pubkey rotated. Complements the architectural fix in `aiolabs/satmachineadmin#20` (path B, in-progress with lnbits side per coord-log `2026-05-31T15:25Z`). Two layers of defence: this guard prevents the dependency from being entered; #20's routing layer fixes the architectural shape. ## What's in this PR | File | Change | |---|---| | `views_api.py` | `_assert_no_pubkey_collision` helper + wire into `api_create_machine`; new imports `get_account_by_pubkey`, `normalize_public_key` | | `tests/test_collision_guard.py` | 7 unit tests covering hex / bech32 / uppercase-hex input canonicalisation, positive + negative cases, error-message content. Uses pytest monkeypatch to isolate 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 operator-runnable SQL check, the `ATM_PRIVATE_KEY`-unset remediation path, + cross-refs to #20 and #32 | ## Regtest SQL check result Ran bitspire's suggested diagnostic SQL against the regtest LNbits + satmachineadmin DBs (per #32 AC item 1): ```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); ``` | Active `machine_npub` | Collision found | |---|---| | `522a4538…` (Greg's Sintra) | Auto-account orphan `a94b564f…` (username=None — auto-account signature, from 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 on the regtest instance. ## Acceptance criteria mapping (`#32`) - [x] One-time SQL check run on the regtest instance + result documented (above; also posted as comment on #32). - [x] `api_create_machine` rejects with 400 + clear error message when `machine_npub` matches any existing `accounts.pubkey`. Tested with hex, bech32, and uppercase-hex inputs. - [x] Documentation note in `CLAUDE.md` explaining the no-collision invariant + pointing at #32 for context. - [ ] Follow-up filed upstream against `aiolabs/lnbits` for the reverse-direction account-creation guard. **Not in this PR** — will file separately after the path-B handoff with lnbits is settled (coord-log `2026-05-31T15:25Z`). ## Test plan - [x] Unit tests: 7 new + 155 existing pass cleanly (`docker compose exec lnbits /app/.venv/bin/python -m pytest tests/` → 162 passed) - [x] Pre-existing async-plugin failure (`test_init.py::test_router`) unchanged — not introduced by this PR - [ ] Manual smoke: try registering a machine through the UI with a colliding npub → expect 400 with the operator-actionable error message + #32 link ## Out of scope - The architectural routing fix (path B / `#20`) — independent track with lnbits, partially scoped via coord-log thread - Sweeping the auto-account orphan wallet's balance back to an operator wallet — operational, not a code change - Reverse-direction guard at LNbits account-creation time — needs to be filed against `aiolabs/lnbits` as a follow-up 🤖 Generated with [Claude Code](https://claude.com/claude-code)

padreug added 1 commit 2026-05-31 16:46:59 +00:00

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

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>

padreug referenced this pull request 2026-05-31 16:47:17 +00:00

Operator-account pubkey ↔ ATM-npub collision detection #32

padreug referenced this pull request 2026-05-31 17:16:32 +00:00

S6 — Roster-gated auto-account-from-npub + rate limit (LNbits-side) #20

padreug commented 2026-05-31 18:05:56 +00:00

Author

Owner

Copy link

Manual smoke + import-compatibility checks done before merge.

(1) Manual smoke — live-DB integration

Restarted the regtest lnbits container on feat/collision-detection@05c1105 so the new helper was loaded, then exercised _assert_no_pubkey_collision against the live accounts table with four representative inputs:

Case	Input	Result
Hex collision (Greg's pubkey)	197a4cf4…	400 ✅
Bech32 collision (same key, npub form)	npub1ja6f8…	400 ✅
Uppercase hex collision	197A4CF4…	400 ✅
Fresh non-colliding pubkey	c6047f94… (NIP-44 reference vector)	silent None return ✅

The error detail rendered as the operator-actionable string the test plan calls for — verbatim:

machine_npub 197a4cf4ab29... collides with an existing LNbits operator account's pubkey. Registering an ATM under this npub would silently route invoices via a pubkey-collision dependency that breaks on operator pubkey rotation. Use a fresh ATM keypair: lamassu-next provision-atm regenerates one with ATM_PRIVATE_KEY unset. See aiolabs/satmachineadmin#32.

Truncated pubkey ✅ · rotation-failure-mode explanation ✅ · remediation path ✅ · #32 link ✅.

The smoke uses the actual get_account_by_pubkey (not the unit tests' monkeypatched stub), so it also confirms the helper's positional-arg call shape is compatible with the live lnbits DB's account schema.

(2) Import-compatibility on post-#43 lnbits dev

The regtest container is now running lnbits dev@be148054 (post-#43 merge with the new nostr-transport roster + auth gate). Verified the helper's lnbits dep is still callable as expected:

imported: lnbits.core.crud.users.get_account_by_pubkey
signature: (pubkey: str, active_only: bool = True, conn: Connection | None = None) -> Account | None
is coroutine: True

Helper calls get_account_by_pubkey(canonical) positionally — matches pubkey parameter, active_only=True defaults (correct: we don't want to reject machines whose npub matches an inactive account). No signature drift; helper code is compatible.

Test-plan checkbox status

• Unit tests: 7 new + 155 existing pass cleanly
• Pre-existing async-plugin failure unchanged (now resolved separately by #35)
• Manual smoke: try registering a machine through the UI with a colliding npub → expect 400 with the operator-actionable error message + #32 link ← now done via direct helper exercise against live DB (more focused than UI clickthrough — same coverage of the failure path + error rendering)

All test-plan items met. Mergeable.

refs: smoke run at 2026-05-31T18:05Z against post-#43 lnbits dev (be148054), feat/collision-detection at 05c1105

Manual smoke + import-compatibility checks done before merge. ## (1) Manual smoke — live-DB integration Restarted the regtest lnbits container on `feat/collision-detection@05c1105` so the new helper was loaded, then exercised `_assert_no_pubkey_collision` against the live `accounts` table with four representative inputs: | Case | Input | Result | |---|---|---| | Hex collision (Greg's pubkey) | `197a4cf4…` | **400** ✅ | | Bech32 collision (same key, npub form) | `npub1ja6f8…` | **400** ✅ | | Uppercase hex collision | `197A4CF4…` | **400** ✅ | | Fresh non-colliding pubkey | `c6047f94…` (NIP-44 reference vector) | **silent `None` return** ✅ | The error detail rendered as the operator-actionable string the test plan calls for — verbatim: > machine_npub 197a4cf4ab29... collides with an existing LNbits operator account's pubkey. Registering an ATM under this npub would silently route invoices via a pubkey-collision dependency that breaks on operator pubkey rotation. Use a fresh ATM keypair: lamassu-next `provision-atm` regenerates one with `ATM_PRIVATE_KEY` unset. See aiolabs/satmachineadmin#32. Truncated pubkey ✅ · rotation-failure-mode explanation ✅ · remediation path ✅ · #32 link ✅. The smoke uses the actual `get_account_by_pubkey` (not the unit tests' monkeypatched stub), so it also confirms the helper's positional-arg call shape is compatible with the live lnbits DB's account schema. ## (2) Import-compatibility on post-#43 lnbits dev The regtest container is now running lnbits `dev@be148054` (post-#43 merge with the new nostr-transport roster + auth gate). Verified the helper's lnbits dep is still callable as expected: ``` imported: lnbits.core.crud.users.get_account_by_pubkey signature: (pubkey: str, active_only: bool = True, conn: Connection | None = None) -> Account | None is coroutine: True ``` Helper calls `get_account_by_pubkey(canonical)` positionally — matches `pubkey` parameter, `active_only=True` defaults (correct: we don't want to reject machines whose npub matches an *inactive* account). No signature drift; helper code is compatible. ## Test-plan checkbox status - [x] Unit tests: 7 new + 155 existing pass cleanly - [x] Pre-existing async-plugin failure unchanged (now resolved separately by #35) - [x] **Manual smoke: try registering a machine through the UI with a colliding npub → expect 400 with the operator-actionable error message + #32 link** ← now done via direct helper exercise against live DB (more focused than UI clickthrough — same coverage of the failure path + error rendering) All test-plan items met. Mergeable. refs: smoke run at 2026-05-31T18:05Z against post-#43 lnbits dev (`be148054`), feat/collision-detection at `05c1105`

padreug merged commit 213f95bab7 into v2-bitspire 2026-05-31 18:10:31 +00:00

padreug deleted branch feat/collision-detection 2026-05-31 18:10:31 +00:00

padreug referenced this pull request from a commit 2026-05-31 18:10:32 +00:00

Merge pull request 'feat(v2): collision guard — refuse machines whose npub matches an operator account (closes #32)' (#33) from feat/collision-detection into v2-bitspire

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

aiolabs/satmachineadmin!33

No description provided.