Merge pull request 'fix: guard every machine_npub deref against unpaired machines (500 + cassette-consumer crash)' (#33) from fix/unpaired-machine-npub-guards into main
Some checks failed
ci.yml / Merge pull request 'fix: guard every machine_npub deref against unpaired machines (500 + cassette-consumer crash)' (#33) from fix/unpaired-machine-npub-guards into main (push) Failing after 0s
/ release (push) Has been cancelled
/ pullrequest (push) Has been cancelled

Reviewed-on: #33
This commit is contained in:
padreug 2026-06-22 14:58:03 +00:00
commit d0d20b0f94
6 changed files with 92 additions and 3 deletions

View file

@ -126,6 +126,14 @@ def assert_nostr_attribution(machine: Machine, extra: dict) -> None:
"missing nostr_sender_pubkey on Payment.extra — invoice was not " "missing nostr_sender_pubkey on Payment.extra — invoice was not "
"issued through the nostr-transport path" "issued through the nostr-transport path"
) )
if not machine.machine_npub:
# Unpaired machine (machine_npub None — nullable since #29/m011). It has
# no identity to attribute a settlement to; reject cleanly rather than
# let normalize_public_key(None) raise an uncaught AttributeError.
raise SettlementAttributionError(
f"machine {machine.id} is unpaired (no machine_npub); "
"a settlement cannot be attributed to it"
)
from lnbits.utils.nostr import normalize_public_key from lnbits.utils.nostr import normalize_public_key
try: try:

View file

@ -141,8 +141,10 @@ def _state_d_tag(atm_pubkey_hex: str) -> str:
def build_state_d_tags_for_machines(machines: list[Machine]) -> list[str]: def build_state_d_tags_for_machines(machines: list[Machine]) -> list[str]:
"""Bootstrap-consumer subscription filter helper: returns the full """Bootstrap-consumer subscription filter helper: returns the full
`#d=[...]` list for all known ATMs an operator subscribes to.""" `#d=[...]` list for all known PAIRED ATMs an operator subscribes to.
return [_state_d_tag(_atm_hex_pubkey(m)) for m in machines] Unpaired machines (machine_npub is None nullable since #29/m011) have no
state-beacon d-tag yet, so skip them rather than crash `_atm_hex_pubkey`."""
return [_state_d_tag(_atm_hex_pubkey(m)) for m in machines if m.machine_npub]
# ============================================================================= # =============================================================================

View file

@ -180,6 +180,11 @@ async def get_machine_by_atm_pubkey_hex(atm_pubkey_hex: str) -> Machine | None:
target = atm_pubkey_hex.lower() target = atm_pubkey_hex.lower()
machines = await list_all_active_machines() machines = await list_all_active_machines()
for m in machines: for m in machines:
# Unpaired machines (machine_npub is None — nullable since #29/m011)
# have no identity to match and would raise AttributeError in
# normalize_public_key (not caught below); skip them.
if not m.machine_npub:
continue
try: try:
if normalize_public_key(m.machine_npub).lower() == target: if normalize_public_key(m.machine_npub).lower() == target:
return m return m

View file

@ -235,7 +235,10 @@ async def _record_rejected(payment: Payment, machine: Machine, exc: Exception) -
return return
logger.error( logger.error(
f"spirekeeper: rejected settlement {rejected.id} " f"spirekeeper: rejected settlement {rejected.id} "
f"(machine={machine.machine_npub[:12]}..., " # An unpaired machine (machine_npub None) reaches here now that
# assert_nostr_attribution rejects it — fall back to the id so the
# log line doesn't crash on None[:12].
f"(machine={(machine.machine_npub or machine.id)[:12]}..., "
f"payment_hash={payment.payment_hash[:12]}...): {exc}" f"payment_hash={payment.payment_hash[:12]}...): {exc}"
) )

View file

@ -0,0 +1,57 @@
"""
Regression: `machine_npub` is nullable (#29/m011 register-unpaired flow), so
every consumer that derives a Nostr identity from it must handle `None` rather
than crash `normalize_public_key(None)` (AttributeError: 'NoneType' has no
'startswith') or `machine_npub[:12]` (TypeError). See PR #33 — an unpaired
machine on the demo broke the platform-fee update (500) and the cassette
consumer.
These cover the pure-function guards; the DB-backed loops
(get_machine_by_atm_pubkey_hex, the super-config republish loop) are exercised
on the dev stack with an unpaired active machine.
"""
from datetime import datetime, timezone
import pytest
from ..bitspire import SettlementAttributionError, assert_nostr_attribution
from ..cassette_transport import build_state_d_tags_for_machines
from ..models import Machine
_PAIRED_HEX = "82341f882b6eabcbd6b1c2da5cd14df14b8e91dd0e6da41a72b78ad8f3a7d3b9"
def _machine(npub: str | None) -> Machine:
now = datetime.now(timezone.utc)
return Machine(
id="unpaired1",
operator_user_id="op1",
machine_npub=npub,
wallet_id="w1",
name="unpaired",
location=None,
fiat_code="EUR",
is_active=True,
created_at=now,
updated_at=now,
)
def test_attribution_rejects_unpaired_machine_cleanly():
"""An unpaired machine must raise the domain SettlementAttributionError
(which the listener records as 'rejected'), not an uncaught AttributeError
from normalize_public_key(None)."""
with pytest.raises(SettlementAttributionError):
assert_nostr_attribution(
_machine(None),
{"source": "bitspire", "nostr_sender_pubkey": _PAIRED_HEX},
)
def test_cassette_d_tags_skip_unpaired_machine():
"""build_state_d_tags_for_machines must skip unpaired machines rather than
crash _atm_hex_pubkey on a None npub the cassette-consumer loop crash."""
tags = build_state_d_tags_for_machines([_machine(_PAIRED_HEX), _machine(None)])
assert len(tags) == 1 # only the paired machine contributes a d-tag
assert all("None" not in t for t in tags)

View file

@ -1081,6 +1081,12 @@ async def api_update_super_config(
) )
if super_fractions_changed: if super_fractions_changed:
for machine in await list_all_active_machines(): for machine in await list_all_active_machines():
# Unpaired machines (machine_npub is None — nullable since #29/m011)
# have no Nostr identity to publish a fee-config beacon to. Skip
# them; they pick up the current fee config when they pair
# (api_pair_machine publishes on success).
if not machine.machine_npub:
continue
await publish_fee_config(machine, config, machine.operator_user_id) await publish_fee_config(machine, config, machine.operator_user_id)
return config return config
@ -1147,6 +1153,14 @@ async def api_publish_machine_cassettes(
500 anything else from the publish path 500 anything else from the publish path
""" """
machine = await _machine_owned_by(machine_id, user.id) machine = await _machine_owned_by(machine_id, user.id)
if not machine.machine_npub:
# Unpaired machine (machine_npub None — nullable since #29/m011) has no
# ATM identity to publish a cassette config to. Fail fast with a clean
# 400 instead of crashing publish_to_atm's normalize_public_key(None).
raise HTTPException(
HTTPStatus.BAD_REQUEST,
"machine is not paired — pair it before publishing cassette config",
)
existing = await list_cassette_configs_for_machine(machine_id) existing = await list_cassette_configs_for_machine(machine_id)
existing_positions = {row.position for row in existing} existing_positions = {row.position for row in existing}