diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..5983318 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,102 @@ +# Changelog + +All notable changes to `le-agent-sdk` are documented here. + +This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +Entries begin at 0.4.0; for earlier history see the +[commit log](https://github.com/refined-element/le-agent-sdk-python/commits/master). + +## [0.4.0] - 2026-07-17 + +Security release. Fixes signature verification silently passing, two payment-budget +bypasses, and unverified relay events — **upgrading is recommended**. + +This release also replaces the package's crypto dependency, which is why it is a +minor bump rather than a patch: see [Dependencies](#dependencies) below. + +### Dependencies + +- **Replaces the `secp256k1` dependency with `coincurve`, which ships prebuilt + wheels.** `secp256k1` required a native build (libsecp256k1 plus a C toolchain) + and failed to install on Windows entirely; even where a build was possible it + frequently produced an install where `import secp256k1` failed, which is what the + verification bug below turned into a silent security hole. `coincurve` provides + the same BIP-340 Schnorr primitives over the same curve with no build step. + + For most users this is transparent — `pip install le-agent-sdk` simply starts + working where it previously did not. Two things to be aware of: + + - If your project imported `secp256k1` itself and relied on this package to pull + it in, it is no longer installed transitively. Declare it directly. + - The signature wire format is unchanged. Events signed by 0.3.x verify under + 0.4.0 and vice versa; the curve, key encoding, and BIP-340 semantics are + identical, only the binding differs. This is covered by cross-implementation + tests against the .NET SDK and the BIP-340 published vectors. + +### Fixed + +- Fixes signature verification silently passing when the crypto backend is + unavailable — upgrade recommended. + `NostrEvent.verify()` returned `True` for any event whose ID matched when the + native secp256k1 library could not be imported. The event ID is a plain SHA-256 + over public fields, so it is attacker-computable and proves nothing about + authenticity — forged capability advertisements and forged attestations under any + pubkey were accepted. Verification now raises `CryptoBackendUnavailableError` + instead of passing. Because the old dependency could not be installed at all on + some platforms, this affected real deployments rather than only misconfigured + ones — and the dependency swap above removes the condition for nearly all of them. +- Fixes `pay_and_access()` ignoring `max_amount_sats`, and budget checks being + skipped for invoices whose amount could not be read — both allowed payments + above the configured limit; upgrade recommended. + `pay_and_access()` never consulted the limit at all, so a client constructed with + `max_amount_sats=100` would pay a 10,000,000-sat invoice. Separately, an invoice + whose amount could not be determined was treated as "no limit applies" and paid. + An amount that cannot be determined is now refused whenever a limit is configured. +- Fixes the BOLT-11 amount parser reading an amount from the invoice data part. + The pattern was not anchored to the human-readable part, so an amountless (i.e. + unbounded) invoice whose data happened to contain `1` was + reported as a small amount and passed the budget check. Amounts are now read only + from the human-readable part, and rounded up rather than truncated so a budget + check is never given an under-reported value. +- Incoming relay events are now signature-verified before use. `discover()`, + `get_attestations()` and `listen_requests()` passed raw relay JSON straight into + the models. Relay lists are caller-configurable and results are merged across + relays, so a single malicious relay could inject events attributed to any pubkey. + Events failing verification are dropped and logged; other relays' results are + unaffected. +- The `User-Agent` sent by `L402ProducerClient` reported `0.1.0` on every release + since 0.1.0. It now tracks the package version. + +### Added + +- `CryptoBackendUnavailableError`, exported from the package root. Subclasses + `RuntimeError`, so existing `except RuntimeError` handlers continue to work. + `Secp256k1UnavailableError` is kept as an alias of it. +- Cross-implementation wire-compatibility tests: events signed by the .NET SDK + (via NBitcoin.Secp256k1) are committed as fixtures and verified on every run, + alongside the BIP-340 published test vectors. + +### Changed + +- `pay_and_access()` accepts a `max_amount_sats` argument to override the + instance-level limit for a single call, matching `access()`. + +### Upgrade notes + +- `pip install le-agent-sdk` no longer needs a C toolchain. If you previously + installed build dependencies (libsecp256k1, build-essential, Visual C++ Build + Tools) solely for this package, they are no longer required. +- If the crypto backend is not importable in your environment, verification now + raises where it previously returned `True`. Any code path that reads events from + relays is affected. This is intentional: the previous result was not a weaker + check, it was no check. +- Callers relying on unknown-amount invoices being paid while `max_amount_sats` is + set will now see `ValueError`. Either set no limit (explicitly opting out of + budget enforcement) or use invoices with an explicit amount. + +### Note on 0.3.3 + +An earlier cut of this work was staged as 0.3.3 and was never published to PyPI. +Its contents are released here as 0.4.0; no 0.3.3 artifact exists. The +`Secp256k1UnavailableError` name originated in that unreleased cut, so no released +version ever exported it — it is aliased anyway for anyone tracking the branch. diff --git a/README.md b/README.md index 7288f5f..a1f0c52 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,23 @@ Discover, request, and settle agent-to-agent services over Nostr with L402 Light pip install le-agent-sdk ``` +> **0.4.0 fixes signature verification silently passing when the crypto backend is +> unavailable, plus two payment-budget bypasses. Upgrading is recommended** — see +> the [changelog](CHANGELOG.md). + +> **0.4.0 replaces the `secp256k1` dependency with [`coincurve`](https://pypi.org/project/coincurve/).** +> `secp256k1` required a native build and could not be installed on Windows at all; +> `coincurve` ships prebuilt wheels, so installation no longer needs a C toolchain. +> The signature wire format is unchanged — events signed by 0.3.x still verify, +> which is covered by cross-implementation tests against the .NET SDK and the +> BIP-340 published vectors. If your own code imported `secp256k1` and relied on +> this package to pull it in, declare it directly. + +Signing, key derivation, and signature verification all need the crypto backend. +If it is not importable they raise `CryptoBackendUnavailableError` (aliased as +`Secp256k1UnavailableError`) rather than degrading to a weaker check — in +particular, verification never reports an unverifiable event as authentic. + ## Quick Start ### Provider: Publish a Service diff --git a/examples/demo_full_loop.py b/examples/demo_full_loop.py index 2a3adc4..d3ff92a 100644 --- a/examples/demo_full_loop.py +++ b/examples/demo_full_loop.py @@ -65,7 +65,7 @@ def generate_throwaway_privkey() -> str: def mock_pubkey_from_privkey(privkey_hex: str) -> str: """Derive a deterministic mock 'pubkey' from a privkey using SHA-256. - This is NOT a real Nostr pubkey derivation (which requires secp256k1). + This is NOT a real Nostr pubkey derivation (which requires coincurve). It is used only in --mock mode to produce consistent, deterministic IDs. """ return hashlib.sha256(bytes.fromhex(privkey_hex)).hexdigest() @@ -109,7 +109,7 @@ def mock_create_event( # --------------------------------------------------------------------------- async def run_mock_demo() -> None: - """Run the full ASA loop with mock data -- no relay, no secp256k1 needed.""" + """Run the full ASA loop with mock data -- no relay, no crypto backend needed.""" print("\n" + "#" * 70) print("# NOSTRWOLFE E2E DEMO -- MOCK MODE") @@ -373,12 +373,12 @@ async def run_live_demo(relay_url: str) -> None: print(f"# Relay: {relay_url}") print("#" + "#" * 69 + "\n") - # Check for secp256k1 + # Check for the BIP-340 crypto backend try: - from le_agent_sdk.nostr.event import _HAS_SECP256K1 - if not _HAS_SECP256K1: - print("WARNING: secp256k1 not installed. Events will be unsigned.") - print("Install with: pip install secp256k1\n") + from le_agent_sdk.nostr.event import _HAS_COINCURVE + if not _HAS_COINCURVE: + print("WARNING: coincurve not installed. Events will be unsigned.") + print("Install with: pip install coincurve\n") except ImportError: pass diff --git a/pyproject.toml b/pyproject.toml index 51056f7..61d26ef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "le-agent-sdk" -version = "0.3.2" +version = "0.4.0" description = "Python SDK for Lightning Enable Agent Service Agreements" requires-python = ">=3.10" license = "MIT" @@ -26,7 +26,10 @@ classifiers = [ dependencies = [ "httpx>=0.24.0", "websockets>=11.0", - "secp256k1>=0.14.0", + # BIP-340 Schnorr over secp256k1. coincurve ships prebuilt wheels for the + # platforms we support; the `secp256k1` package it replaced in 0.4.0 required + # a native build toolchain and could not be installed on Windows at all. + "coincurve>=18.0", "l402-requests>=0.1.0", ] diff --git a/src/le_agent_sdk/__init__.py b/src/le_agent_sdk/__init__.py index 946a700..f89ff41 100644 --- a/src/le_agent_sdk/__init__.py +++ b/src/le_agent_sdk/__init__.py @@ -4,7 +4,11 @@ from le_agent_sdk.models.request import AgentServiceRequest from le_agent_sdk.models.agreement import AgentServiceAgreement from le_agent_sdk.models.attestation import AgentAttestation -from le_agent_sdk.nostr.event import NostrEvent +from le_agent_sdk.nostr.event import ( + CryptoBackendUnavailableError, + NostrEvent, + Secp256k1UnavailableError, +) from le_agent_sdk.nostr.relay import RelayClient from le_agent_sdk.nostr.tags import TagParser from le_agent_sdk.l402.client import L402Client, L402ProducerClient @@ -17,6 +21,10 @@ "AgentServiceAgreement", "AgentAttestation", "NostrEvent", + "CryptoBackendUnavailableError", + # Pre-0.4.0 alias of CryptoBackendUnavailableError, kept for callers + # catching it by name. + "Secp256k1UnavailableError", "RelayClient", "TagParser", "L402Client", @@ -24,4 +32,4 @@ "AgentManager", ] -__version__ = "0.3.2" +__version__ = "0.4.0" diff --git a/src/le_agent_sdk/agent/manager.py b/src/le_agent_sdk/agent/manager.py index b576205..15ea7b7 100644 --- a/src/le_agent_sdk/agent/manager.py +++ b/src/le_agent_sdk/agent/manager.py @@ -7,6 +7,7 @@ from __future__ import annotations import asyncio +import logging from typing import Any, AsyncIterator, Optional from le_agent_sdk.l402.client import L402Client, L402ProducerClient @@ -14,10 +15,12 @@ from le_agent_sdk.models.attestation import AgentAttestation from le_agent_sdk.models.capability import AgentCapability from le_agent_sdk.models.request import AgentServiceRequest -from le_agent_sdk.nostr.event import NostrEvent +from le_agent_sdk.nostr.event import CryptoBackendUnavailableError, NostrEvent from le_agent_sdk.nostr.relay import RelayClient from le_agent_sdk.nostr.tags import TagParser +logger = logging.getLogger(__name__) + class AgentManager: """Main entry point for agent operations. @@ -64,6 +67,39 @@ def pubkey(self) -> str: self._pubkey = NostrEvent.pubkey_from_private_key(self.private_key) return self._pubkey + @staticmethod + def _is_event_authentic(event: dict[str, Any]) -> bool: + """Check a relay-supplied event's signature before trusting its contents. + + Relay URLs are caller-configurable and results are merged across relays, + so without this a single malicious or compromised relay could inject + events attributed to any pubkey — forged capability ads, forged + attestations inflating an agent's reputation. + + A verification failure drops only the offending event: one bad relay in + the pool must not be able to fail an otherwise good query. A RuntimeError + (crypto backend unavailable) is left to propagate — that is an environment + fault affecting every event, and silently returning zero results would + misrepresent it as "nothing found". + """ + if NostrEvent.verify(event): + return True + + logger.warning( + "Dropping Nostr event %.16s... (kind=%s) from pubkey %.16s...: " + "signature verification failed. The relay may be malicious or " + "misbehaving.", + event.get("id", ""), + event.get("kind", ""), + event.get("pubkey", ""), + ) + return False + + @classmethod + def _filter_authentic(cls, events: list[dict[str, Any]]) -> list[dict[str, Any]]: + """Keep only events with a valid signature under their claimed pubkey.""" + return [event for event in events if cls._is_event_authentic(event)] + async def _publish_to_relays(self, event: dict[str, Any]) -> str: """Publish an event to all configured relays. @@ -183,7 +219,10 @@ async def discover( ) raw_events = await self._query_relays([nostr_filter], timeout=timeout) - return [AgentCapability.from_nostr_event(e) for e in raw_events] + return [ + AgentCapability.from_nostr_event(e) + for e in self._filter_authentic(raw_events) + ] async def publish_capability(self, capability: AgentCapability) -> str: """Publish a capability advertisement to relays. @@ -296,7 +335,14 @@ async def listen_requests( if event_id and event_id not in seen_ids: seen_ids.add(event_id) reconnect_attempts = 0 # Reset on successful message + if not self._is_event_authentic(event_data): + continue yield AgentServiceRequest.from_nostr_event(event_data) + except CryptoBackendUnavailableError: + # Environment fault, not a relay fault: reconnecting cannot + # fix a missing crypto backend. Surface it immediately + # instead of burning the reconnect budget on it. + raise except Exception: reconnect_attempts += 1 if reconnect_attempts > max_reconnect_attempts: @@ -521,7 +567,10 @@ async def get_attestations( ) raw_events = await self._query_relays([nostr_filter], timeout=timeout) - return [AgentAttestation.from_nostr_event(e) for e in raw_events] + return [ + AgentAttestation.from_nostr_event(e) + for e in self._filter_authentic(raw_events) + ] async def get_reputation_score( self, diff --git a/src/le_agent_sdk/l402/client.py b/src/le_agent_sdk/l402/client.py index 3dad462..b7d3f63 100644 --- a/src/le_agent_sdk/l402/client.py +++ b/src/le_agent_sdk/l402/client.py @@ -22,6 +22,26 @@ logger = logging.getLogger(__name__) +def _sdk_version() -> str: + """Version string for the User-Agent header. + + Read from the package __version__ rather than hardcoded, so it cannot drift + from the real version (it previously advertised 0.1.0 from a 0.3.x release, + which made server-side version telemetry misleading). Imported lazily to + avoid a circular import: the package __init__ imports this module. + + __version__ is used in preference to importlib.metadata because it reflects + the code actually running, which is the point of the header; installed + distribution metadata can be stale or absent in a source checkout. + """ + try: + from le_agent_sdk import __version__ + + return __version__ + except Exception: # pragma: no cover - defensive: UA must never break a request + return "unknown" + + @dataclass(frozen=True) class L402Challenge: """Parsed L402 challenge from a WWW-Authenticate header.""" @@ -80,6 +100,25 @@ class MppChallenge: re.IGNORECASE, ) +# BOLT-11 human-readable part: "ln" + currency prefix + optional amount. +# Anchored with a terminating $ so it only ever matches a complete HRP — +# never a fragment of the bech32 data part. Longer currency prefixes are +# listed first because Python's alternation takes the first match. +_BOLT11_HRP_RE = re.compile( + r"^ln(?:bcrt|bc|tbs|tb|sb)(?P\d+)?(?P[munp])?$", + re.IGNORECASE, +) + +# Amount multipliers expressed in pico-BTC, so amounts stay exact integers. +# 1 BTC = 10^12 pico-BTC = 10^8 sats, therefore 1 sat = 10^4 pico-BTC. +_PICO_BTC_MULTIPLIERS = { + "m": 10**9, # milli-BTC + "u": 10**6, # micro-BTC + "n": 10**3, # nano-BTC + "p": 1, # pico-BTC + "": 10**12, # no multiplier => whole BTC +} + def parse_l402_challenge(headers: dict[str, str]) -> Optional[L402Challenge]: """Extract an L402 challenge from response headers. @@ -230,34 +269,49 @@ def _ensure_client(self) -> httpx.AsyncClient: def _decode_invoice_amount_sats(invoice: str) -> Optional[int]: """Extract the amount in satoshis from a BOLT-11 invoice string. - Returns None if the amount cannot be parsed. + Returns: + The amount in satoshis, rounded UP to the next whole sat, or None if + the invoice encodes no amount or cannot be parsed. None means + "amount unknown" — it never means "no amount limit applies". Callers + enforcing a budget MUST refuse a None (see _check_amount_against_max). + + Security: + The amount is read only from the human-readable part (HRP), which is + everything before the final bech32 separator. Scanning the whole + string would let the amount be matched from inside the data part of + an amountless (i.e. unbounded) invoice, reporting a small bogus + amount that passes a budget check. """ - # BOLT-11: starts with "ln" then network (bc/tb/etc), then optional amount - # Amount is encoded as: where multipliers are: - # m=milli, u=micro, n=nano, p=pico (of BTC) - inv_lower = invoice.lower() - # Strip "lightning:" prefix if present + inv_lower = invoice.lower().strip() + # Strip "lightning:" URI prefix if present if inv_lower.startswith("lightning:"): inv_lower = inv_lower[10:] - match = re.match(r"ln\w+?(\d+)([munp])1", inv_lower) + # Per BIP-173 the separator is the LAST "1" in the string: the bech32 + # data charset excludes "1", so any earlier "1" belongs to the HRP. + separator = inv_lower.rfind("1") + if separator < 0: + return None + hrp = inv_lower[:separator] + + # HRP grammar: "ln" + currency prefix + optional (amount + multiplier). + # Longer prefixes must precede their own prefixes in the alternation. + match = _BOLT11_HRP_RE.match(hrp) if not match: return None - amount_num = int(match.group(1)) - multiplier = match.group(2) - # Convert to satoshis (1 BTC = 100_000_000 sats) - multiplier_map = { - "m": 100_000_00, # milli-BTC = 100,000 sats (0.001 BTC) - "u": 100_00, # micro-BTC = 100 sats (0.000001 BTC) - "n": 0.01, # nano-BTC = 0.01 sats - "p": 0.00001, # pico-BTC = 0.00001 sats - } - # milli = 10^-3 BTC = 10^5 sats - btc_multipliers = {"m": 1e-3, "u": 1e-6, "n": 1e-9, "p": 1e-12} - btc_amount = amount_num * btc_multipliers[multiplier] - sats = int(btc_amount * 1e8) - return sats + amount_digits = match.group("amount") + if not amount_digits: + return None # amountless invoice: payer chooses => unknown + + # Integer-only math in pico-BTC; floats would round a budget-critical + # value in the unsafe direction. + amount_pico = int(amount_digits) * _PICO_BTC_MULTIPLIERS[match.group("multiplier") or ""] + if amount_pico <= 0: + return None + + # 1 sat = 10_000 pico-BTC. Round UP: never under-report to a budget check. + return -(-amount_pico // 10_000) @staticmethod def _validate_preimage(preimage: str) -> bool: @@ -270,6 +324,110 @@ def _validate_preimage(preimage: str) -> bool: except ValueError: return False + def _check_amount_against_max( + self, + challenge: L402Challenge | MppChallenge, + effective_max: Optional[int], + ) -> None: + """Enforce the payment ceiling before any invoice reaches the wallet. + + The invariant is: an amount that cannot be determined is refused. + A budget is a guarantee ("never pay more than N"), and an invoice whose + amount cannot be proven <= N cannot be paid without breaking it. The + wallet callback is arbitrary caller-supplied code, so handing it an + unbounded invoice delegates an unbounded spend. + + No ceiling configured (None) means the caller explicitly opted out of + budget enforcement, so any invoice — known or unknown — is allowed + through; refusing there would add no safety and break the documented + "None means no limit" contract. + + Raises: + ValueError: If the invoice exceeds the ceiling, or if a ceiling is + configured and the amount cannot be determined. + """ + if effective_max is None: + return + + invoice_sats = self._decode_invoice_amount_sats(challenge.invoice) + + if invoice_sats is None: + raise ValueError( + "Invoice amount could not be determined, and a maximum of " + f"{effective_max} sats is configured. Refusing to pay: an " + "invoice with no verifiable amount cannot be checked against a " + "budget and would hand an unbounded payment to the wallet " + f"callback. Invoice: {challenge.invoice[:40]}..." + ) + + if invoice_sats > effective_max: + raise ValueError( + f"Invoice amount ({invoice_sats} sats) exceeds maximum allowed " + f"({effective_max} sats). Invoice: {challenge.invoice[:40]}..." + ) + + async def _execute_payment( + self, + challenge: L402Challenge | MppChallenge, + pay_invoice_callback: Any, + effective_max: Optional[int], + url: str, + ) -> str: + """Check the budget, pay the invoice, and validate the preimage. + + The single payment path shared by access() and pay_and_access() so the + budget ceiling can never apply to one entry point but not the other. + + Returns: + The validated preimage. + + Raises: + ValueError: If the amount is over budget/undeterminable, or the + callback returns a malformed preimage. + RuntimeError: If the payment callback itself fails. + """ + # Budget gate FIRST: nothing reaches the wallet before this passes. + self._check_amount_against_max(challenge, effective_max) + + try: + preimage = await pay_invoice_callback(challenge.invoice) + except Exception as exc: + logger.error( + "Error in pay_invoice_callback for URL %r: %s", url, exc, exc_info=True + ) + raise RuntimeError(f"Payment callback failed: {exc}") from exc + + if not self._validate_preimage(preimage): + logger.error( + "Invalid preimage returned from pay callback: expected 64-char hex, " + "got %r (length=%d)", + preimage[:20] if isinstance(preimage, str) else type(preimage), + len(preimage) if isinstance(preimage, str) else 0, + ) + raise ValueError( + f"Invalid preimage from payment callback: expected 64-character hex string, " + f"got length {len(preimage) if isinstance(preimage, str) else 'N/A'}" + ) + + # Only L402 credentials are cacheable (keyed by macaroon); MPP has none. + if isinstance(challenge, L402Challenge): + self._cache[challenge.macaroon] = preimage + + protocol = "MPP" if isinstance(challenge, MppChallenge) else "L402" + logger.info("%s payment succeeded for %s", protocol, url) + logger.debug("%s preimage (first 8 chars): %.8s...", protocol, preimage) + + return preimage + + @staticmethod + def _build_auth_header( + challenge: L402Challenge | MppChallenge, preimage: str + ) -> str: + """Build the Authorization header value for a paid challenge.""" + if isinstance(challenge, MppChallenge): + return f'Payment method="lightning", preimage="{preimage}"' + return f"L402 {challenge.macaroon}:{preimage}" + async def access( self, url: str, @@ -324,49 +482,14 @@ async def access( retry_response = await client.request(method, url, headers=headers, **kwargs) return retry_response - # Check invoice amount against limit + # Budget check, payment, and preimage validation (shared path) effective_max = max_amount_sats if max_amount_sats is not None else self._max_amount_sats - if effective_max is not None: - invoice_sats = self._decode_invoice_amount_sats(challenge.invoice) - if invoice_sats is not None and invoice_sats > effective_max: - raise ValueError( - f"Invoice amount ({invoice_sats} sats) exceeds maximum allowed " - f"({effective_max} sats). Invoice: {challenge.invoice[:40]}..." - ) - - # Pay the invoice with error handling on the callback - try: - preimage = await self._pay_callback(challenge.invoice) - except Exception as exc: - logger.error("pay_invoice_callback failed: %s", exc) - raise RuntimeError(f"Payment callback failed: {exc}") from exc - - # Validate preimage format - if not self._validate_preimage(preimage): - logger.error( - "Invalid preimage returned from pay callback: expected 64-char hex, " - "got %r (length=%d)", - preimage[:20] if isinstance(preimage, str) else type(preimage), - len(preimage) if isinstance(preimage, str) else 0, - ) - raise ValueError( - f"Invalid preimage from payment callback: expected 64-character hex string, " - f"got length {len(preimage) if isinstance(preimage, str) else 'N/A'}" - ) - - # Build the correct Authorization header based on challenge type - if isinstance(challenge, MppChallenge): - auth_header = f'Payment method="lightning", preimage="{preimage}"' - logger.info("MPP payment succeeded for %s", url) - logger.debug("MPP preimage (first 8 chars): %.8s...", preimage) - else: - self._cache[challenge.macaroon] = preimage - auth_header = f"L402 {challenge.macaroon}:{preimage}" - logger.info("L402 payment succeeded for %s", url) - logger.debug("L402 preimage (first 8 chars): %.8s...", preimage) + preimage = await self._execute_payment( + challenge, self._pay_callback, effective_max, url + ) # Retry the request with credentials, with retry+backoff - headers["Authorization"] = auth_header + headers["Authorization"] = self._build_auth_header(challenge, preimage) max_retries = 3 last_exc: Optional[Exception] = None @@ -401,6 +524,7 @@ async def pay_and_access( pay_invoice_callback: Any, method: str = "GET", headers: Optional[dict[str, str]] = None, + max_amount_sats: Optional[int] = None, **kwargs: Any, ) -> httpx.Response: """Full L402 flow: request, get 402, pay invoice, retry with token. @@ -410,10 +534,16 @@ async def pay_and_access( pay_invoice_callback: Async callable(invoice: str) -> preimage: str. method: HTTP method. headers: Optional request headers. + max_amount_sats: Override max payment amount for this request. + Falls back to the instance-level max_amount_sats. **kwargs: Additional httpx request kwargs. Returns: The final HTTP response after payment. + + Raises: + ValueError: If the invoice amount exceeds max_amount_sats, or if a + limit is configured and the amount cannot be determined. """ headers = dict(headers or {}) client = self._ensure_client() @@ -438,39 +568,13 @@ async def pay_and_access( retry_response = await client.request(method, url, headers=headers, **kwargs) return retry_response - try: - preimage = await pay_invoice_callback(challenge.invoice) - except Exception as exc: - logger.error( - "Error in pay_invoice_callback during pay_and_access for URL %r: %s", - url, - exc, - exc_info=True, - ) - raise RuntimeError( - "Payment callback failed during pay_and_access; see logs for details" - ) from exc - - # Validate preimage format before constructing credentials - if not self._validate_preimage(preimage): - logger.error( - "Invalid preimage returned from pay callback in pay_and_access: " - "expected 64-char hex, got %r (length=%d)", - preimage[:20] if isinstance(preimage, str) else type(preimage), - len(preimage) if isinstance(preimage, str) else 0, - ) - raise ValueError( - f"Invalid preimage from payment callback: expected 64-character hex string, " - f"got length {len(preimage) if isinstance(preimage, str) else 'N/A'}" - ) - - # Build the correct Authorization header based on challenge type - if isinstance(challenge, MppChallenge): - headers["Authorization"] = f'Payment method="lightning", preimage="{preimage}"' - else: - self._cache[challenge.macaroon] = preimage - headers["Authorization"] = f"L402 {challenge.macaroon}:{preimage}" + # Budget check, payment, and preimage validation (shared path) + effective_max = max_amount_sats if max_amount_sats is not None else self._max_amount_sats + preimage = await self._execute_payment( + challenge, pay_invoice_callback, effective_max, url + ) + headers["Authorization"] = self._build_auth_header(challenge, preimage) retry_response = await client.request(method, url, headers=headers, **kwargs) return retry_response @@ -542,7 +646,7 @@ def _ensure_client(self) -> httpx.AsyncClient: "X-Api-Key": self._api_key, "Content-Type": "application/json", "Accept": "application/json", - "User-Agent": "LE-Agent-SDK-Python/0.1.0", + "User-Agent": f"LE-Agent-SDK-Python/{_sdk_version()}", } self._client = httpx.AsyncClient(headers=headers, **self._httpx_kwargs) return self._client diff --git a/src/le_agent_sdk/nostr/event.py b/src/le_agent_sdk/nostr/event.py index a14000d..105a36f 100644 --- a/src/le_agent_sdk/nostr/event.py +++ b/src/le_agent_sdk/nostr/event.py @@ -1,7 +1,7 @@ """Nostr event builder and signing (NIP-01). Handles event creation, ID computation (SHA-256 of canonical serialization), -and Schnorr signing (BIP-340) via the secp256k1 library. +and Schnorr signing (BIP-340) via the coincurve library. """ from __future__ import annotations @@ -11,13 +11,43 @@ import time from typing import Any, Optional -# Signing is optional — if secp256k1 is not installed, events are created unsigned. +# coincurve provides BIP-340 Schnorr over secp256k1 and ships prebuilt wheels +# for every platform we support, so a normal `pip install` is sufficient. +# +# The import is still guarded. A wheel can be missing for an unusual +# platform/interpreter combination, and an install can be pruned after the fact, +# so the failure mode has to be defined rather than incidental. Every operation +# that depends on it — sign(), pubkey_from_private_key() and verify() — raises +# when it is missing. None of them degrade to a weaker check. Only +# building/serializing unsigned events works without it. try: - import secp256k1 + import coincurve - _HAS_SECP256K1 = True + _HAS_COINCURVE = True except ImportError: - _HAS_SECP256K1 = False + _HAS_COINCURVE = False + + +class CryptoBackendUnavailableError(RuntimeError): + """Raised when an operation needs the BIP-340 backend but it is unimportable. + + Subclasses RuntimeError so existing `except RuntimeError` handlers keep + working. Distinguishable so that callers can tell this environment fault + apart from an operational error (e.g. a relay disconnect) and avoid + retrying something that will never succeed. + """ + + +# The backend moved from `secp256k1` to `coincurve` in 0.4.0, which dates the +# old name. It is kept as an alias so `except Secp256k1UnavailableError` keeps +# working: the curve is still secp256k1, only the binding changed. Prefer +# CryptoBackendUnavailableError in new code. +Secp256k1UnavailableError = CryptoBackendUnavailableError + +_MISSING_BACKEND_HINT = ( + "coincurve is required for BIP-340 Schnorr operations. " + "Install with: pip install coincurve" +) class NostrEvent: @@ -56,21 +86,17 @@ def pubkey_from_private_key(private_key_hex: str) -> str: Returns: 32-byte x-only public key as hex string. """ - if not _HAS_SECP256K1: - raise RuntimeError( - "secp256k1 library is required for key derivation. " - "Install with: pip install secp256k1" + if not _HAS_COINCURVE: + raise CryptoBackendUnavailableError( + f"Key derivation is unavailable: {_MISSING_BACKEND_HINT}" ) privkey_bytes = bytes.fromhex(private_key_hex) if len(privkey_bytes) != 32: raise ValueError( f"Private key must be 32 bytes, got {len(privkey_bytes)} bytes" ) - keypair = secp256k1.PrivateKey(privkey_bytes) - # secp256k1 public key is 33 bytes (compressed); strip the prefix byte - pubkey_bytes = keypair.pubkey.serialize(compressed=True) - # x-only pubkey is the last 32 bytes of the compressed key (drop 0x02/0x03 prefix) - return pubkey_bytes[1:].hex() + # BIP-340 keys are x-only: the x coordinate with the y parity dropped. + return coincurve.PublicKeyXOnly.from_secret(privkey_bytes).format().hex() @staticmethod def sign(event_id_hex: str, private_key_hex: str) -> str: @@ -79,37 +105,54 @@ def sign(event_id_hex: str, private_key_hex: str) -> str: Returns: 64-byte signature as hex string. """ - if not _HAS_SECP256K1: - raise RuntimeError( - "secp256k1 library is required for signing. " - "Install with: pip install secp256k1" + if not _HAS_COINCURVE: + raise CryptoBackendUnavailableError( + f"Signing is unavailable: {_MISSING_BACKEND_HINT}" ) privkey_bytes = bytes.fromhex(private_key_hex) if len(privkey_bytes) != 32: raise ValueError( f"Private key must be 32 bytes, got {len(privkey_bytes)} bytes" ) + # NIP-01 signs the 32-byte event id directly; it is already a digest and + # must not be hashed again. sign_schnorr takes the message unhashed. msg_bytes = bytes.fromhex(event_id_hex) - keypair = secp256k1.PrivateKey(privkey_bytes) - sig = keypair.schnorr_sign(msg_bytes, bip340tag=b"", raw=True) - return sig.hex() + keypair = coincurve.PrivateKey(privkey_bytes) + return keypair.sign_schnorr(msg_bytes).hex() @staticmethod def verify(event: dict[str, Any]) -> bool: """Verify a Nostr event's ID and signature. + The ID alone is NOT authentication: it is a plain SHA-256 over public + fields with no secret input, so anyone can compute a matching ID for an + event they forged. Authenticity comes solely from the BIP-340 signature. + Returns: - True if valid, False otherwise. + True only if the ID matches AND the signature is a valid BIP-340 + signature over that ID under the claimed pubkey. False otherwise. + + Raises: + CryptoBackendUnavailableError: If coincurve is unavailable, so the + signature cannot be checked. This fails closed and loudly, + consistent with sign() and pubkey_from_private_key(). It is a + RuntimeError subclass and never silently passes. """ # Verify ID computed_id = NostrEvent.compute_id(event) if computed_id != event.get("id", ""): return False - if not _HAS_SECP256K1: - # Cannot verify signature without secp256k1; only ID was checked - return True + # Raised before the try below, so a missing backend can never be caught + # and turned into a plain False — an unverifiable event must not be + # reported as merely invalid. + if not _HAS_COINCURVE: + raise CryptoBackendUnavailableError( + "Signature verification is unavailable. Refusing to treat the " + "event as verified: the event ID is a plain hash of public " + f"fields and proves nothing about authenticity. {_MISSING_BACKEND_HINT}" + ) pubkey_hex = event.get("pubkey", "") sig_hex = event.get("sig", "") @@ -119,11 +162,12 @@ def verify(event: dict[str, Any]) -> bool: try: msg_bytes = bytes.fromhex(event["id"]) sig_bytes = bytes.fromhex(sig_hex) - # Reconstruct compressed pubkey (prepend 0x02) - pubkey_bytes = bytes.fromhex("02" + pubkey_hex) - pubkey = secp256k1.PublicKey(pubkey_bytes, raw=True) - return pubkey.schnorr_verify(msg_bytes, sig_bytes, bip340tag=b"", raw=True) + # BIP-340 verification takes the 32-byte x-only pubkey directly. + pubkey = coincurve.PublicKeyXOnly(bytes.fromhex(pubkey_hex)) + return bool(pubkey.verify(sig_bytes, msg_bytes)) except Exception: + # Malformed pubkey/signature/id — not authentic. Distinct from a + # missing backend, which is raised above and never reaches here. return False @staticmethod diff --git a/tests/fixtures/dotnet_signed_events.json b/tests/fixtures/dotnet_signed_events.json new file mode 100644 index 0000000..9d0912d --- /dev/null +++ b/tests/fixtures/dotnet_signed_events.json @@ -0,0 +1,82 @@ +{ + "_comment": [ + "Nostr events signed by the .NET SDK (le-agent-sdk-dotnet) using", + "NBitcoin.Secp256k1 3.2.0 — an implementation with no code in common with", + "Python's coincurve. They are committed so the Python suite proves wire", + "compatibility on every run without needing a .NET toolchain in CI.", + "", + "These are a consensus artifact of the protocol, not test scaffolding: if a", + "change makes them stop verifying, this SDK has broken signature", + "compatibility with every other implementation on the network. Regenerate", + "them only when the .NET SDK's own wire format deliberately changes, never", + "to make a failing test pass.", + "", + "Generated from LightningEnable.AgentSdk.Nostr.NostrEvent.Sign() via", + "NostrEvent.GetPublicKey() / ComputeId(), .NET 8." + ], + "events": [ + { + "id": "fdf6fbd62ea5c7b6b0ba5e7e4cf5e99b1d1f1795535b159cb6cb15644e45f361", + "pubkey": "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + "created_at": 1700000000, + "kind": 38400, + "tags": [ + [ + "d", + "svc-dotnet" + ], + [ + "s", + "ai" + ] + ], + "content": "Independent .NET-signed capability advertisement", + "sig": "bf080521c02b2bbce3f7c360deb2353230b6292fa602c19a031b9041c24661c8351ceb0d494932f2d8d739f30cdb4942b52d0d6d0eae09b7f68520b0e6d41ee1", + "_label": "ascii capability advertisement (kind 38400)" + }, + { + "id": "66609ad11d3d018f116e5872ff147956156a13ff010b81178eb181e22bd0eb80", + "pubkey": "dff1d77f2a671c5f36183726db2341be58feae1da2deced843240f7b502ba659", + "created_at": 1700000001, + "kind": 38401, + "tags": [ + [ + "p", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + ], + [ + "amount", + "1000" + ] + ], + "content": "service request from the .NET SDK", + "sig": "f62cd11fe887ba732532a1e8bab05cc6dd52c860785e932f6c97a5e7b3b60d2a375cec99002c5e90a13586bb483b27b8cc44b2bcc9f780d73470d641868a4b76", + "_label": "service request with tags (kind 38401)" + }, + { + "id": "9a9679bfd0e5e2c375fa6c6f95e6b84bda4d3809eed0b3d5552c541a9003a3d7", + "pubkey": "dd308afec5777e13121fa72b9cc1b7cc0139715309b086c960e18fd969774eb8", + "created_at": 1700000002, + "kind": 1, + "tags": [ + [ + "t", + "ünïcödé" + ] + ], + "content": "unicode ünïcödé 日本語 🚀 astral + \"quotes\" and \\backslash\\", + "sig": "1d0a2c63df9a2d392f759137215888a6213e6862bdc317d27add823f08288b22a0387a465a936cd2a624b10b32bc846037178b04be036985ae9b885e3cb81aa1", + "_label": "unicode + astral plane + escaped quotes/backslashes (kind 1)" + }, + { + "id": "03164c3bd6496a6554d09ecbceb9b2c8cc3af3e34aaf376f33e9269c61d74fbc", + "pubkey": "25d1dff95105f5253c4022f628a996ad3a0d95fbf21d468a1b33f8c160d8f517", + "created_at": 1700000003, + "kind": 0, + "tags": [], + "content": "", + "sig": "ef9eff4c10c9e95685e847a96d9d2c30a6082c18603f6bc57d2f0fa1978f72b226178bf51d19e6e4c970424e78b026ea9c139955cd153a6d3985c8e937f520a1", + "_label": "empty content, no tags (kind 0)" + } + ] +} diff --git a/tests/test_agent_manager.py b/tests/test_agent_manager.py index 9381b48..263626d 100644 --- a/tests/test_agent_manager.py +++ b/tests/test_agent_manager.py @@ -8,6 +8,7 @@ from le_agent_sdk.agent.manager import AgentManager from le_agent_sdk.models.agreement import AgentServiceAgreement from le_agent_sdk.models.capability import AgentCapability, AgentPricing +from le_agent_sdk.nostr.event import NostrEvent class TestAgentManagerInit: @@ -32,6 +33,13 @@ def test_pubkey_without_private_key_raises(self): class TestAgentManagerDiscover: @pytest.mark.asyncio async def test_discover_returns_capabilities(self): + """Authentic events are parsed into capabilities. + + discover() verifies signatures before parsing, so verification is stubbed + to isolate the parsing behaviour under test. Signing real fixtures here + would require the crypto backend. The drop-on-forgery path is + covered in tests/test_security_regressions.py. + """ sample_events = [ { "id": "ev1", @@ -56,7 +64,8 @@ async def test_discover_returns_capabilities(self): mgr = AgentManager() with patch.object(mgr, "_query_relays", new_callable=AsyncMock) as mock_query: mock_query.return_value = sample_events - caps = await mgr.discover(categories=["ai"]) + with patch.object(NostrEvent, "verify", return_value=True): + caps = await mgr.discover(categories=["ai"]) assert len(caps) == 2 assert caps[0].service_id == "svc-a" diff --git a/tests/test_interop.py b/tests/test_interop.py new file mode 100644 index 0000000..8b4436e --- /dev/null +++ b/tests/test_interop.py @@ -0,0 +1,258 @@ +"""Cross-implementation wire-compatibility tests. + +Nostr events are a wire protocol: the event ID is a consensus value and the +signature must verify under any conforming implementation. A signature scheme +that only agrees with itself is worthless — a round-trip test would pass just as +happily on a private, incompatible curve. + +So these tests check this SDK against implementations it shares no code with: + + * BIP-340 published test vectors (the standard itself). + * Events signed by the .NET SDK (le-agent-sdk-dotnet) via NBitcoin.Secp256k1, + committed under tests/fixtures/ so no .NET toolchain is needed here. + +The fixtures are protocol artifacts, not scaffolding. If they stop verifying, +this SDK has broken compatibility with the network — regenerate them only when +the .NET SDK's wire format deliberately changes, never to make a test pass. +""" + +import json +from pathlib import Path + +import pytest + +from le_agent_sdk.nostr.event import NostrEvent + +FIXTURES = Path(__file__).parent / "fixtures" + + +def _load_dotnet_events(): + with open(FIXTURES / "dotnet_signed_events.json", encoding="utf-8") as fh: + return json.load(fh)["events"] + + +def _strip(event: dict) -> dict: + """Drop harness-only annotations, leaving the on-the-wire event.""" + return {k: v for k, v in event.items() if not k.startswith("_")} + + +DOTNET_EVENTS = _load_dotnet_events() +DOTNET_IDS = [e["_label"] for e in DOTNET_EVENTS] + + +class TestDotNetSignedEventsVerify: + """.NET (NBitcoin.Secp256k1) -> Python (coincurve).""" + + @pytest.mark.parametrize("event", DOTNET_EVENTS, ids=DOTNET_IDS) + def test_dotnet_event_id_agrees(self, event): + """Both implementations must derive the same ID from the same event. + + Covers the canonical NIP-01 serialization, including the non-ASCII and + astral-plane escaping rules the two languages implement separately. + """ + event = _strip(event) + assert NostrEvent.compute_id(event) == event["id"] + + @pytest.mark.parametrize("event", DOTNET_EVENTS, ids=DOTNET_IDS) + def test_dotnet_signature_verifies(self, event): + """A signature made by the .NET SDK must verify here.""" + assert NostrEvent.verify(_strip(event)) is True + + def test_fixture_actually_covers_non_ascii(self): + """Guard the guard: the unicode case must not silently vanish. + + Non-ASCII is where independently written serializers realistically + diverge, so losing that fixture would gut these tests without failing + anything. + """ + assert any( + any(ord(ch) > 0xFFFF for ch in e["content"]) for e in DOTNET_EVENTS + ), "no astral-plane content in fixtures" + assert any( + any(ord(ch) > 127 for ch in e["content"]) for e in DOTNET_EVENTS + ), "no non-ASCII content in fixtures" + + +class TestDotNetSignedEventsRejectTampering: + """The cross-impl fixtures must not verify once altered. + + Without these, a verify() that returned True unconditionally would pass + every test above. + """ + + def test_tampered_content_breaks_dotnet_event(self): + event = _strip(DOTNET_EVENTS[0]) + event["content"] = "tampered by a relay in transit" + assert NostrEvent.verify(event) is False + + def test_swapped_signature_breaks_dotnet_event(self): + event = _strip(DOTNET_EVENTS[0]) + event["sig"] = _strip(DOTNET_EVENTS[1])["sig"] + assert NostrEvent.verify(event) is False + + def test_dotnet_event_reattributed_to_another_pubkey_rejected(self): + """Reattribution with a recomputed ID: the ID check passes, sig must not.""" + event = _strip(DOTNET_EVENTS[0]) + event["pubkey"] = _strip(DOTNET_EVENTS[1])["pubkey"] + event["id"] = NostrEvent.compute_id(event) + + assert NostrEvent.compute_id(event) == event["id"], "ID check would pass" + assert NostrEvent.verify(event) is False + + +class TestPythonSignedEventsMatchDotNetKeys: + """Python -> .NET, verified through values .NET produced. + + Signatures are randomized per BIP-340 aux, so Python's signature bytes are + not expected to equal .NET's. Key derivation is deterministic, so agreement + on the pubkey is checkable here; the signature direction (Python-signed + events verifying under .NET) is exercised out-of-band against the .NET SDK. + """ + + @pytest.mark.parametrize( + "private_key,expected_pubkey", + [ + # (private key, x-only pubkey as derived by .NET's GetPublicKey()). + # These also match the BIP-340 published vectors. + ( + "0000000000000000000000000000000000000000000000000000000000000001", + "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + ), + ( + "b7e151628aed2a6abf7158809cf4f3c762e7160f38b4da56a784d9045190cfef", + "dff1d77f2a671c5f36183726db2341be58feae1da2deced843240f7b502ba659", + ), + ( + "c90fdaa22168c234c4c6628b80dc1cd129024e088a67cc74020bbea63b14e5c9", + "dd308afec5777e13121fa72b9cc1b7cc0139715309b086c960e18fd969774eb8", + ), + ( + "0b432b2677937381aef05bb02a66ecd012773062cf3fa2549e44f58ed2401710", + "25d1dff95105f5253c4022f628a996ad3a0d95fbf21d468a1b33f8c160d8f517", + ), + ], + ) + def test_pubkey_derivation_matches_dotnet(self, private_key, expected_pubkey): + """x-only derivation must drop y-parity the same way in both SDKs. + + One of these keys has odd y (0b432b26...), so a binding that leaked parity + into the x-only key would disagree here and silently produce events + attributed to a pubkey no one else computes. Keep that vector: drop it and + the parity coverage silently falls to zero. + """ + assert NostrEvent.pubkey_from_private_key(private_key) == expected_pubkey + + def test_locally_signed_event_verifies_under_dotnet_derived_pubkey(self): + """Sign here, verify against the pubkey .NET says the key has.""" + private_key = "b7e151628aed2a6abf7158809cf4f3c762e7160f38b4da56a784d9045190cfef" + event = NostrEvent.create( + kind=38400, + content="signed by python, keyed by .NET's derivation", + tags=[["d", "svc-1"]], + private_key=private_key, + created_at=1700000000, + ) + assert event["pubkey"] == ( + "dff1d77f2a671c5f36183726db2341be58feae1da2deced843240f7b502ba659" + ) + assert NostrEvent.verify(event) is True + + +class TestBip340PublishedVectors: + """Conformance to the BIP-340 standard itself. + + From the BIP-340 reference test vectors (indices 0-4). These pin the SDK to + the spec rather than to any one library's behaviour, and would catch a + backend swap that changed message-hashing semantics (e.g. a binding that + hashes the message again before signing — the event ID is already a digest + and must be signed as-is). + + Provenance matters here: a vector copied out of the library under test would + only prove the library agrees with itself. Each signature below was + independently reproduced by NBitcoin.Secp256k1 (the .NET SDK's backend) + signing the same key/message/aux_rand — BIP-340 signing is deterministic + given aux_rand, so two unrelated implementations emitting identical bytes is + what establishes these as the spec's values. + """ + + # (index, pubkey, message, signature, expected) + VECTORS = [ + ( + 0, + "f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9", + "0000000000000000000000000000000000000000000000000000000000000000", + "e907831f80848d1069a5371b402410364bdf1c5f8307b0084c55f1ce2dca8215" + "25f66a4a85ea8b71e482a74f382d2ce5ebeee8fdb2172f477df4900d310536c0", + True, + ), + ( + 1, + "dff1d77f2a671c5f36183726db2341be58feae1da2deced843240f7b502ba659", + "243f6a8885a308d313198a2e03707344a4093822299f31d0082efa98ec4e6c89", + "6896bd60eeae296db48a229ff71dfe071bde413e6d43f917dc8dcf8c78de3341" + "8906d11ac976abccb20b091292bff4ea897efcb639ea871cfa95f6de339e4b0a", + True, + ), + ( + 2, + "dd308afec5777e13121fa72b9cc1b7cc0139715309b086c960e18fd969774eb8", + "7e2d58d8b3bcdf1abadec7829054f90dda9805aab56c77333024b9d0a508b75c", + "5831aaeed7b44bb74e5eab94ba9d4294c49bcf2a60728d8b4c200f50dd313c1b" + "ab745879a5ad954a72c45a91c3a51d3c7adea98d82f8481e0e1e03674a6f3fb7", + True, + ), + ( + 3, + "25d1dff95105f5253c4022f628a996ad3a0d95fbf21d468a1b33f8c160d8f517", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "7eb0509757e246f19449885651611cb965ecc1a187dd51b64fda1edc9637d5ec" + "97582b9cb13db3933705b32ba982af5af25fd78881ebb32771fc5922efc66ea3", + True, + ), + ( + 4, + "d69c3509bb99e412e68b0fe8544e72837dfa30746d8be2aa65975f29d22dc7b9", + "4df3c3f68fcc83b27e9d42c90431a72499f17875c81a599b566c9889b9696703", + "00000000000000000000003b78ce563f89a0ed9414f5aa28ad0d96d6795f9c63" + "76afb1548af603b3eb45c9f8207dee1060cb71c04e80f593060b07d28308d7f4", + True, + ), + ] + + @pytest.mark.parametrize( + "index,pubkey,message,signature,expected", + VECTORS, + ids=[f"bip340-vector-{v[0]}" for v in VECTORS], + ) + def test_bip340_vector(self, index, pubkey, message, signature, expected): + """Drive the vector through verify() by presenting it as an event. + + verify() checks the ID before the signature, so the vector's message is + placed in `id`: that is exactly how NIP-01 uses it — the 32-byte value + signed. The ID check is neutralised by monkeypatching compute_id, since + these vectors are raw BIP-340 messages, not NIP-01 serializations. + """ + event = {"id": message, "pubkey": pubkey, "sig": signature} + + original = NostrEvent.compute_id + try: + NostrEvent.compute_id = staticmethod(lambda e: e["id"]) + assert NostrEvent.verify(event) is expected + finally: + NostrEvent.compute_id = original + + def test_vector_signature_rejected_under_wrong_pubkey(self): + """Negative control: the vectors must not verify under any key.""" + _, _, message, signature, _ = self.VECTORS[1] + event = { + "id": message, + "pubkey": self.VECTORS[2][1], # a different, valid pubkey + "sig": signature, + } + + original = NostrEvent.compute_id + try: + NostrEvent.compute_id = staticmethod(lambda e: e["id"]) + assert NostrEvent.verify(event) is False + finally: + NostrEvent.compute_id = original diff --git a/tests/test_security_regressions.py b/tests/test_security_regressions.py new file mode 100644 index 0000000..a78f60b --- /dev/null +++ b/tests/test_security_regressions.py @@ -0,0 +1,516 @@ +"""Security regression tests for fail-open / budget-bypass defects. + +Each test in this module corresponds to a confirmed vulnerability. They are +written to fail against the pre-fix code and pass after the fix. + +Covered: + 1. NostrEvent.verify() fail-open when the crypto backend is unavailable. + 2. L402Client.pay_and_access() ignoring max_amount_sats. + 3. Budget check skipped when the invoice amount is unparseable. + 4. Reputation scoring ignoring out-of-range ratings (already correct — locked in). + 5. AgentManager trusting unverified relay events. +""" + +from unittest.mock import AsyncMock, patch + +import pytest + +from le_agent_sdk.agent.manager import AgentManager +from le_agent_sdk.l402.client import L402Client +from le_agent_sdk.models.attestation import AgentAttestation +from le_agent_sdk.nostr import event as event_module +from le_agent_sdk.nostr.event import ( + CryptoBackendUnavailableError, + NostrEvent, + Secp256k1UnavailableError, +) + +# --- Fixtures / helpers ----------------------------------------------------- + +# A well-formed event whose `id` is a genuine SHA-256 of its public fields but +# whose `sig` is garbage. An attacker can compute `id` offline (no secret input), +# so ID-only validation is not authentication. +def _forged_event(pubkey: str = "de" * 32) -> dict: + event = { + "pubkey": pubkey, + "created_at": 1700000000, + "kind": 38400, + "tags": [["d", "svc-a"], ["s", "ai"]], + "content": "Totally legit service", + } + event["id"] = NostrEvent.compute_id(event) # attacker-computable + event["sig"] = "00" * 64 # not a valid BIP-340 signature + return event + + +class TestVerifyFailsClosedWithoutCryptoBackend: + """Finding 1: verify() returned True when the crypto backend was unimportable. + + The backend moved from secp256k1 to coincurve in 0.4.0. coincurve ships + prebuilt wheels, so the "absent backend" case is far less likely to occur by + accident than it was — but "unlikely" is not "impossible" (an unusual + platform with no wheel, or a pruned install), and the fail-open bug this + class covers is exactly what happens when an unlikely branch is left + undefined. The behaviour is still pinned. + """ + + def test_verify_raises_when_backend_unavailable(self): + """Missing backend must be loud, not a silent pass. + + Consistent with sign()/pubkey_from_private_key(), which already raise. + """ + with patch.object(event_module, "_HAS_COINCURVE", False): + with pytest.raises(RuntimeError, match="coincurve"): + NostrEvent.verify(_forged_event()) + + def test_verify_raises_specific_error_type(self): + """The raised type must stay catchable under both names.""" + with patch.object(event_module, "_HAS_COINCURVE", False): + with pytest.raises(CryptoBackendUnavailableError): + NostrEvent.verify(_forged_event()) + # The pre-0.4.0 name is an alias, so existing handlers keep working. + with patch.object(event_module, "_HAS_COINCURVE", False): + with pytest.raises(Secp256k1UnavailableError): + NostrEvent.verify(_forged_event()) + + def test_forged_event_does_not_verify_as_true(self): + """A forged event must never come back as verified. + + Regardless of whether the backend is installed, the one outcome that must + be impossible is a `True` return for an event with a bogus signature. + """ + with patch.object(event_module, "_HAS_COINCURVE", False): + try: + result = NostrEvent.verify(_forged_event()) + except RuntimeError: + return # fail-closed: acceptable + assert result is not True, "forged event verified as authentic" + + def test_verify_still_rejects_id_mismatch_before_dep_check(self): + """A tampered ID is rejected without needing the backend.""" + event = _forged_event() + event["content"] = "tampered after id was computed" + with patch.object(event_module, "_HAS_COINCURVE", False): + assert NostrEvent.verify(event) is False + + def test_sign_raises_when_backend_unavailable(self): + with patch.object(event_module, "_HAS_COINCURVE", False): + with pytest.raises(CryptoBackendUnavailableError, match="coincurve"): + NostrEvent.sign("ab" * 32, "01" * 32) + + def test_pubkey_derivation_raises_when_backend_unavailable(self): + with patch.object(event_module, "_HAS_COINCURVE", False): + with pytest.raises(CryptoBackendUnavailableError, match="coincurve"): + NostrEvent.pubkey_from_private_key("01" * 32) + + +class TestVerifyAcceptsGenuineRejectsForged: + """The signature path must discriminate, not just refuse everything. + + Fixing a fail-open bug by failing everything closed would be no fix. These + tests exercise the real BIP-340 path with genuine signatures. + + Before 0.4.0 these signed fixtures with coincurve and shimmed it over the + secp256k1 binding, because secp256k1 needs a native build and was routinely + unimportable — so the signature path could not otherwise be tested at all. + coincurve is now the runtime backend, so the shim is gone and these drive + the real production code path end to end. Cross-implementation agreement is + covered separately in test_interop.py; the concern here is that verify() + tells genuine and forged apart. + """ + + @staticmethod + def _sign(private_hex: str, content: str = "genuine") -> dict: + """Build a genuinely signed event through the SDK's own signing path.""" + return NostrEvent.create( + kind=38400, + content=content, + tags=[["d", "svc-a"]], + private_key=private_hex, + created_at=1700000000, + ) + + def test_genuine_signed_event_verifies(self): + assert NostrEvent.verify(self._sign("01" * 32)) is True + + def test_forged_signature_rejected(self): + event = self._sign("01" * 32) + event["sig"] = "00" * 64 + assert NostrEvent.verify(event) is False + + def test_tampered_content_rejected(self): + event = self._sign("01" * 32) + event["content"] = "tampered" + assert NostrEvent.verify(event) is False + + def test_event_reattributed_to_another_pubkey_rejected(self): + """The core attack: swap the pubkey and recompute a valid ID. + + The ID check alone passes here — it is just a hash of public fields. + Only the signature check stops it. + """ + event = self._sign("01" * 32) + + event["pubkey"] = NostrEvent.pubkey_from_private_key("02" * 32) + event["id"] = NostrEvent.compute_id(event) # ID now matches again + + assert NostrEvent.compute_id(event) == event["id"], "ID check would pass" + assert NostrEvent.verify(event) is False, "reattributed event was accepted" + + def test_signature_from_different_key_rejected(self): + event = self._sign("01" * 32) + event["sig"] = NostrEvent.sign(event["id"], "03" * 32) + assert NostrEvent.verify(event) is False + + def test_sign_verify_round_trips_through_public_api(self): + """create() -> verify() must hold for the documented entry point.""" + event = NostrEvent.create( + kind=38400, + content="round trip", + tags=[["d", "svc-a"], ["s", "ai"]], + private_key="04" * 32, + ) + assert event["pubkey"] == NostrEvent.pubkey_from_private_key("04" * 32) + assert NostrEvent.verify(event) is True + + +class TestPayAndAccessRespectsMaxAmount: + """Finding 2: pay_and_access never read self._max_amount_sats.""" + + @pytest.mark.asyncio + async def test_pay_and_access_rejects_invoice_over_instance_limit(self): + # 10,000,000 sats == 100m (milli-BTC) invoice, limit is 100 sats. + expensive = "lnbc100m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rq" + paid = [] + + async def pay_callback(invoice): + paid.append(invoice) + return "ab" * 32 + + client = L402Client(max_amount_sats=100) + + with patch.object(client, "_ensure_client") as mock_ensure: + mock_http = AsyncMock() + mock_http.request.return_value = _make_402(expensive) + mock_ensure.return_value = mock_http + + with pytest.raises(ValueError, match="exceeds maximum"): + await client.pay_and_access("https://x.test/r", pay_callback) + + assert paid == [], "wallet callback was invoked for an over-budget invoice" + + @pytest.mark.asyncio + async def test_pay_and_access_allows_invoice_under_limit(self): + cheap = "lnbc10u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rq" # 1000 sats + + async def pay_callback(invoice): + return "ab" * 32 + + client = L402Client(max_amount_sats=5000) + + with patch.object(client, "_ensure_client") as mock_ensure: + mock_http = AsyncMock() + mock_http.request.side_effect = [_make_402(cheap), _make_ok()] + mock_ensure.return_value = mock_http + + resp = await client.pay_and_access("https://x.test/r", pay_callback) + + assert resp.status_code == 200 + + @pytest.mark.asyncio + async def test_pay_and_access_per_call_override_beats_instance_limit(self): + cheap = "lnbc10u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rq" # 1000 sats + + async def pay_callback(invoice): + return "ab" * 32 + + client = L402Client(max_amount_sats=100_000) + + with patch.object(client, "_ensure_client") as mock_ensure: + mock_http = AsyncMock() + mock_http.request.return_value = _make_402(cheap) + mock_ensure.return_value = mock_http + + with pytest.raises(ValueError, match="exceeds maximum"): + await client.pay_and_access( + "https://x.test/r", pay_callback, max_amount_sats=10 + ) + + +class TestUnknownAmountIsRefused: + """Finding 3: unparseable amount was read as 'no limit applies'.""" + + def test_amountless_invoice_decodes_to_none(self): + amountless = "lnbc1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypq" + assert L402Client._decode_invoice_amount_sats(amountless) is None + + def test_amountless_invoice_is_not_misparsed_from_data_part(self): + """The HRP amount must not be matched from inside the bech32 data part. + + Pre-fix the regex was unanchored, so an amountless (unbounded) invoice + whose data contained '1' reported a small bogus amount and + sailed through the budget check. + """ + assert L402Client._decode_invoice_amount_sats("lnbc1pabc9u1def") is None + assert L402Client._decode_invoice_amount_sats("lnbc1pvjl5p1uez") is None + + @pytest.mark.asyncio + async def test_access_refuses_unparseable_invoice_when_budget_set(self): + garbage = "not-a-parseable-bolt11-invoice" + paid = [] + + async def pay_callback(invoice): + paid.append(invoice) + return "ab" * 32 + + client = L402Client(pay_invoice_callback=pay_callback, max_amount_sats=1000) + + with patch.object(client, "_ensure_client") as mock_ensure: + mock_http = AsyncMock() + mock_http.request.return_value = _make_402(garbage) + mock_ensure.return_value = mock_http + + with pytest.raises(ValueError, match="amount"): + await client.access("https://x.test/r") + + assert paid == [], "unbounded invoice was handed to the wallet callback" + + @pytest.mark.asyncio + async def test_pay_and_access_refuses_unparseable_invoice_when_budget_set(self): + garbage = "not-a-parseable-bolt11-invoice" + paid = [] + + async def pay_callback(invoice): + paid.append(invoice) + return "ab" * 32 + + client = L402Client(max_amount_sats=1000) + + with patch.object(client, "_ensure_client") as mock_ensure: + mock_http = AsyncMock() + mock_http.request.return_value = _make_402(garbage) + mock_ensure.return_value = mock_http + + with pytest.raises(ValueError, match="amount"): + await client.pay_and_access("https://x.test/r", pay_callback) + + assert paid == [] + + @pytest.mark.asyncio + async def test_unparseable_invoice_still_paid_when_no_budget_configured(self): + """No budget configured == caller explicitly accepted unbounded payment. + + Refusing here would break the documented `None means no limit` contract + without adding safety: with no limit, a known 10M-sat invoice is paid too. + """ + garbage = "not-a-parseable-bolt11-invoice" + paid = [] + + async def pay_callback(invoice): + paid.append(invoice) + return "ab" * 32 + + client = L402Client(pay_invoice_callback=pay_callback, max_amount_sats=None) + + with patch.object(client, "_ensure_client") as mock_ensure: + mock_http = AsyncMock() + mock_http.request.side_effect = [_make_402(garbage), _make_ok()] + mock_ensure.return_value = mock_http + + resp = await client.access("https://x.test/r") + + assert resp.status_code == 200 + assert paid == [garbage] + + +class TestReputationIgnoresOutOfRangeRatings: + """Finding 4: verified as already-correct. Locked in against regression.""" + + @pytest.mark.asyncio + async def test_out_of_range_ratings_excluded_from_average(self): + attestations = [ + AgentAttestation(rating=5), + AgentAttestation(rating=5), + AgentAttestation(rating=9999), # forged/out-of-range + AgentAttestation(rating=0), # unparsed/missing + AgentAttestation(rating=-5), + ] + mgr = AgentManager() + with patch.object(mgr, "get_attestations", new_callable=AsyncMock) as mock_get: + mock_get.return_value = attestations + score = await mgr.get_reputation_score("pub") + + assert score == 5.0 + + @pytest.mark.asyncio + async def test_no_valid_ratings_returns_none(self): + mgr = AgentManager() + with patch.object(mgr, "get_attestations", new_callable=AsyncMock) as mock_get: + mock_get.return_value = [AgentAttestation(rating=0)] + score = await mgr.get_reputation_score("pub") + + assert score is None + + +class TestManagerVerifiesRelayEvents: + """Finding 5: raw relay JSON flowed into models with no verification.""" + + @pytest.mark.asyncio + async def test_discover_drops_events_failing_verification(self): + mgr = AgentManager() + good, bad = _forged_event("aa" * 32), _forged_event("bb" * 32) + + with patch.object(mgr, "_query_relays", new_callable=AsyncMock) as mock_query: + mock_query.return_value = [good, bad] + with patch.object( + NostrEvent, "verify", side_effect=lambda e: e["pubkey"] == "aa" * 32 + ): + caps = await mgr.discover() + + assert len(caps) == 1, "unverified event was not dropped" + assert caps[0].pubkey == "aa" * 32 + + @pytest.mark.asyncio + async def test_get_attestations_drops_events_failing_verification(self): + mgr = AgentManager() + good, bad = _forged_event("aa" * 32), _forged_event("bb" * 32) + + with patch.object(mgr, "_query_relays", new_callable=AsyncMock) as mock_query: + mock_query.return_value = [good, bad] + with patch.object( + NostrEvent, "verify", side_effect=lambda e: e["pubkey"] == "aa" * 32 + ): + atts = await mgr.get_attestations("aa" * 32) + + assert len(atts) == 1 + assert atts[0].pubkey == "aa" * 32 + + @pytest.mark.asyncio + async def test_discover_drops_all_when_every_event_is_forged(self): + """One malicious relay must not be able to inject attributed events.""" + mgr = AgentManager() + + with patch.object(mgr, "_query_relays", new_callable=AsyncMock) as mock_query: + mock_query.return_value = [_forged_event(), _forged_event("cc" * 32)] + with patch.object(NostrEvent, "verify", return_value=False): + caps = await mgr.discover() + + assert caps == [] + + @pytest.mark.asyncio + async def test_verification_error_propagates(self): + """A missing native dep must surface, not silently empty the results.""" + mgr = AgentManager() + + with patch.object(mgr, "_query_relays", new_callable=AsyncMock) as mock_query: + mock_query.return_value = [_forged_event()] + with patch.object( + NostrEvent, "verify", side_effect=RuntimeError("coincurve required") + ): + with pytest.raises(RuntimeError, match="coincurve"): + await mgr.discover() + + @pytest.mark.asyncio + async def test_listen_requests_drops_forged_events(self): + """The streaming ingestion point must verify too, not just the queries.""" + good = _forged_event("aa" * 32) + good["kind"] = 38401 + bad = _forged_event("bb" * 32) + bad["kind"] = 38401 + + mgr = AgentManager(private_key="01" * 32) + received = [] + + # The forged event is streamed FIRST: without verification it is what the + # caller receives, so ordering is what gives this test its teeth. + with patch.object( + type(mgr), "pubkey", property(lambda self: "aa" * 32) + ), patch.object(NostrEvent, "verify", side_effect=lambda e: e["pubkey"] == "aa" * 32): + with patch( + "le_agent_sdk.agent.manager.RelayClient", _FakeRelayClient([bad, good]) + ): + async for req in mgr.listen_requests(): + received.append(req) + if len(received) >= 1: + break + + assert len(received) == 1 + assert received[0].pubkey == "aa" * 32, "forged request was yielded to caller" + + @pytest.mark.asyncio + async def test_listen_requests_surfaces_missing_dep_without_reconnect_storm(self): + """A missing dep must not be mistaken for a relay fault and retried.""" + event = _forged_event("aa" * 32) + event["kind"] = 38401 + + mgr = AgentManager(private_key="01" * 32) + fake_relay_cls = _FakeRelayClient([event]) + + with patch.object( + type(mgr), "pubkey", property(lambda self: "aa" * 32) + ), patch.object( + NostrEvent, "verify", side_effect=CryptoBackendUnavailableError("coincurve missing") + ): + with patch("le_agent_sdk.agent.manager.RelayClient", fake_relay_cls): + with pytest.raises(CryptoBackendUnavailableError): + async for _ in mgr.listen_requests(): + pass + + # Exactly one connect: the error escaped instead of driving reconnects. + assert fake_relay_cls.connect_count == 1, ( + f"missing dep triggered {fake_relay_cls.connect_count} connects " + "(reconnect storm) instead of surfacing immediately" + ) + + +# --- Relay double ----------------------------------------------------------- + + +def _FakeRelayClient(events: list[dict]): + """Build a RelayClient stand-in class that streams `events` once. + + Returns a class (not an instance) because the manager constructs its own + RelayClient objects. Connects are counted on the class so a reconnect storm + is observable. + """ + + class _Fake: + connect_count = 0 + + async def connect(self, url): + type(self).connect_count += 1 + + async def subscribe(self, filters): + return None + + async def listen(self): + for event in events: + yield "EVENT", ("sub-id", event) + # Stream ends: modelled as a disconnect, which is what a real relay + # closing the socket looks like to the manager. + raise ConnectionError("relay closed the stream") + + async def close(self): + return None + + return _Fake + + +# --- HTTP response doubles -------------------------------------------------- + + +def _make_402(invoice: str): + """Build a fake 402 response carrying an L402 challenge.""" + resp = AsyncMock() + resp.status_code = 402 + resp.headers = { + "WWW-Authenticate": f'L402 macaroon="AGIAJEem", invoice="{invoice}"' + } + return resp + + +def _make_ok(): + resp = AsyncMock() + resp.status_code = 200 + resp.headers = {} + return resp