diff --git a/CLAUDE.md b/CLAUDE.md index acf23cc..9d2af64 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -219,38 +219,6 @@ 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 diff --git a/tests/test_collision_guard.py b/tests/test_collision_guard.py deleted file mode 100644 index 0f1a236..0000000 --- a/tests/test_collision_guard.py +++ /dev/null @@ -1,124 +0,0 @@ -""" -Tests for `views_api._assert_no_pubkey_collision` (aiolabs/satmachineadmin#32). - -Defends against the silent-drop failure mode reproduced on 2026-05-30T21:33Z: -Greg's operator account pubkey had been seeded identical to the Sintra ATM's -machine_npub, which masked the routing problem until Greg's pubkey rotated -during the bunker migration — then `auto-account-from-npub` fired for the -orphaned ATM npub and the cash-out invoice silently landed on a fresh -auto-account wallet. - -The guard refuses to register a machine whose npub matches any LNbits -operator account's `accounts.pubkey`, so this state cannot be entered -through the satmachineadmin UI in the first place. - -Monkeypatches `views_api.get_account_by_pubkey` to avoid needing a live -LNbits DB; this matches the assertion-style of tests/test_nostr_attribution -(both isolate the assertion function for unit-testability). -""" - -import asyncio -from types import SimpleNamespace - -import pytest - -from .. import views_api -from ..views_api import _assert_no_pubkey_collision - -# Canonical x-only pubkey for the integer 1 secret (matches NIP-44 reference vector). -_PUBKEY_HEX = "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798" -# Bech32 form of the same pubkey — operators may enter either form in the UI. -_PUBKEY_NPUB = "npub10xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqpkge6d" - - -def _fake_account(pubkey: str = _PUBKEY_HEX): - """Account-shaped duck-typed object. _assert_no_pubkey_collision only - cares whether get_account_by_pubkey returns non-None; the returned - shape doesn't matter beyond that.""" - return SimpleNamespace(id="op1", username="alice", pubkey=pubkey) - - -def _patch_lookup(monkeypatch, return_value): - """Replace `views_api.get_account_by_pubkey` with an async stub that - captures the canonical-hex argument the guard normalised to and - returns the configured value.""" - captured = {} - - async def fake_lookup(pubkey: str): - captured["called_with"] = pubkey - return return_value - - monkeypatch.setattr(views_api, "get_account_by_pubkey", fake_lookup) - return captured - - -class TestCollisionDetected: - """Positive cases: machine_npub collides with an operator account's - pubkey. Each form (hex / bech32 / uppercase) must normalise to the - same canonical lookup + raise the same 400.""" - - def test_collision_with_hex_input_raises(self, monkeypatch): - _patch_lookup(monkeypatch, return_value=_fake_account()) - with pytest.raises(Exception) as exc: - asyncio.run(_assert_no_pubkey_collision(_PUBKEY_HEX)) - assert exc.value.status_code == 400 - assert "collides with an existing LNbits operator account" in exc.value.detail - assert "aiolabs/satmachineadmin#32" in exc.value.detail - - def test_collision_with_bech32_input_raises(self, monkeypatch): - """Operator may enter `npub1...` in the UI; the guard must - canonicalise to hex BEFORE the lookup, otherwise a colliding - npub-form input would silently miss the hex-stored - accounts.pubkey row.""" - captured = _patch_lookup(monkeypatch, return_value=_fake_account()) - with pytest.raises(Exception) as exc: - asyncio.run(_assert_no_pubkey_collision(_PUBKEY_NPUB)) - assert exc.value.status_code == 400 - # The bech32 input must be canonicalised to lowercase hex before the lookup. - assert captured["called_with"] == _PUBKEY_HEX - - def test_collision_with_uppercase_hex_input_raises(self, monkeypatch): - """Hex inputs from manual entry / paste can land uppercase; the - guard's `normalize_public_key().lower()` should bring it to the - canonical lowercase hex that get_account_by_pubkey itself also - lowercases internally.""" - captured = _patch_lookup(monkeypatch, return_value=_fake_account()) - with pytest.raises(Exception) as exc: - asyncio.run(_assert_no_pubkey_collision(_PUBKEY_HEX.upper())) - assert exc.value.status_code == 400 - assert captured["called_with"] == _PUBKEY_HEX - - -class TestNoCollision: - """Negative cases: machine_npub does not match any account → guard - returns silently, machine creation can proceed.""" - - def test_no_collision_returns_silently(self, monkeypatch): - _patch_lookup(monkeypatch, return_value=None) - # Should NOT raise. - asyncio.run(_assert_no_pubkey_collision(_PUBKEY_HEX)) - - def test_no_collision_bech32_form_returns_silently(self, monkeypatch): - captured = _patch_lookup(monkeypatch, return_value=None) - asyncio.run(_assert_no_pubkey_collision(_PUBKEY_NPUB)) - # The lookup still gets called with the canonicalised hex form. - assert captured["called_with"] == _PUBKEY_HEX - - -class TestErrorMessage: - """The 400 detail must be operator-actionable: explains the failure, - points at the issue, and gives the remediation path.""" - - def test_error_includes_truncated_pubkey(self, monkeypatch): - _patch_lookup(monkeypatch, return_value=_fake_account()) - with pytest.raises(Exception) as exc: - asyncio.run(_assert_no_pubkey_collision(_PUBKEY_HEX)) - # First 12 chars of the canonical lowercase hex, followed by an ellipsis. - assert _PUBKEY_HEX[:12] in exc.value.detail - - def test_error_includes_remediation_hint(self, monkeypatch): - _patch_lookup(monkeypatch, return_value=_fake_account()) - with pytest.raises(Exception) as exc: - asyncio.run(_assert_no_pubkey_collision(_PUBKEY_HEX)) - assert "lamassu-next" in exc.value.detail - assert "ATM_PRIVATE_KEY" in exc.value.detail diff --git a/views_api.py b/views_api.py index 6889803..66d9e76 100644 --- a/views_api.py +++ b/views_api.py @@ -9,10 +9,8 @@ from http import HTTPStatus from fastapi import APIRouter, Depends, HTTPException from lnbits.core.crud import get_wallet -from lnbits.core.crud.users import get_account_by_pubkey from lnbits.core.models import User from lnbits.decorators import check_super_user, check_user_exists -from lnbits.utils.nostr import normalize_public_key from .cassette_transport import ( CassetteTransportError, @@ -107,46 +105,6 @@ async def _assert_wallet_owned_by(wallet_id: str, user_id: str) -> None: ) -async def _assert_no_pubkey_collision(machine_npub: str) -> None: - """Defence-in-depth: refuse to register a machine whose npub matches - any LNbits operator account's pubkey. - - Such a collision causes lnbits' nostr-transport `auth.py:resolve_ - nostr_auth` to route inbound kind-21000 RPCs from the ATM directly - to that operator's wallet — works by coincidence, but breaks silently - the moment the operator's pubkey rotates (because the auto-account- - from-npub flow then fires for the ATM's now-orphaned npub, and the - invoice lands on a fresh auto-account wallet instead). Reproducer: - Greg's Sintra silent-drop on 2026-05-30T21:33Z. See - aiolabs/satmachineadmin#32 for the failure mode + this guard's - design rationale. - - Path B (`#20` roster-lookup) is the architectural fix at the - routing layer; this guard prevents new operators from inadvertently - setting up the collision in the first place. Two layers of defence. - - Idempotent on the same caller re-attempting machine creation with - the same npub (the second attempt hits the dca_machines.machine_npub - UNIQUE on m001, not this guard — they only collide with operator- - account pubkeys, not other machine npubs). - """ - canonical = normalize_public_key(machine_npub).lower() - matching = await get_account_by_pubkey(canonical) - if matching is not None: - raise HTTPException( - HTTPStatus.BAD_REQUEST, - ( - f"machine_npub {canonical[:12]}... collides with an " - f"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." - ), - ) - - # ============================================================================= # Machines # ============================================================================= @@ -157,7 +115,6 @@ async def api_create_machine( data: CreateMachineData, user: User = Depends(check_user_exists) ) -> Machine: await _assert_wallet_owned_by(data.wallet_id, user.id) - await _assert_no_pubkey_collision(data.machine_npub) machine = await create_machine(user.id, data) return machine