From 8d42886ab5235a04932f6c362153569c83f0d459 Mon Sep 17 00:00:00 2001 From: Padreug Date: Tue, 26 May 2026 10:23:36 +0200 Subject: [PATCH] test(lnbits): integration coverage for handleReply forgery-rejection wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to commit 0dcbe44 (closes aiolabs/lamassu-next#49) addressing two review notes from the bitspire session: 1. Integration test gap. The four unit tests on isAuthenticServerEvent cover the predicate in isolation — a refactor that dropped the `if (!isAuthenticServerEvent(...)) return` line from handleReply would still pass them silently. Adds two integration tests that exercise the full handleReply wiring through a mock NostrClient that captures the subscribe callback: - Forged event injected through the callback → pre-registered pending entry's resolve is NOT called (asserts handleReply short-circuited). - Legitimate server-signed reply (NIP-44 v2 encrypted with real keys) → pending entry's resolve IS called with the decoded payload (positive sanity). 2. `@internal` JSDoc on isAuthenticServerEvent. The helper is exported only so the unit tests can reach it directly; production callers should go through handleReply. The annotation makes the intent explicit and discourages accidental wider use. 6/6 tests pass. Workspace typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/lnbits/src/__tests__/client.test.ts | 164 ++++++++++++++++++- packages/lnbits/src/client.ts | 4 + 2 files changed, 167 insertions(+), 1 deletion(-) diff --git a/packages/lnbits/src/__tests__/client.test.ts b/packages/lnbits/src/__tests__/client.test.ts index faf619c..eaa8aa8 100644 --- a/packages/lnbits/src/__tests__/client.test.ts +++ b/packages/lnbits/src/__tests__/client.test.ts @@ -15,7 +15,12 @@ import { type Event as NostrEvent, type UnsignedEvent, } from 'nostr-tools' -import { isAuthenticServerEvent } from '../client.js' +import { + encryptContentV2, + type MachineIdentity, + type NostrClient, +} from '@bitSpire/nostr-client' +import { LnbitsClient, isAuthenticServerEvent } from '../client.js' const KIND_LNBITS_RPC = 21000 @@ -127,3 +132,160 @@ describe('isAuthenticServerEvent', () => { expect(isAuthenticServerEvent(tampered, serverPubkey)).toBe(false) }) }) + +/** + * Integration test for `LnbitsClient.handleReply` wiring. + * + * The unit tests above prove `isAuthenticServerEvent` is correct in + * isolation. This block proves `handleReply` actually invokes it before + * resolving a pending RPC — a refactor that drops the guard would fail + * this test even if the unit tests still pass. + * + * Approach: build a `LnbitsClient` against a minimal mock `NostrClient` + * that captures the `onEvent` handler from `startReplyListener`. Inject + * a forged event through that handler and assert the pre-registered + * pending entry's `resolve` is never called. For the positive sanity, + * encrypt a real `LnbitsRpcResponse` with NIP-44 v2 and confirm + * `resolve` does fire. + */ +describe('LnbitsClient.handleReply wiring', () => { + function makeMockNostr(): { + nostr: NostrClient + triggerEvent: (ev: NostrEvent) => void + } { + let captured: ((ev: NostrEvent) => void) | null = null + const nostr = { + subscribe( + _filters: unknown, + opts: { onEvent: (ev: NostrEvent) => void }, + ): string { + captured = opts.onEvent + return 'mock-sub-id' + }, + publish: async () => {}, + unsubscribe: async () => {}, + } as unknown as NostrClient + return { + nostr, + triggerEvent: (ev) => { + if (!captured) throw new Error('handleReply not wired yet') + captured(ev) + }, + } + } + + function makeIdentity(): MachineIdentity { + const sk = generateSecretKey() + return { + privateKey: sk, + publicKey: getPublicKey(sk), + npub: '', + } + } + + function setupClient(): { + client: LnbitsClient + serverIdentity: MachineIdentity + recipientIdentity: MachineIdentity + triggerEvent: (ev: NostrEvent) => void + } { + const serverIdentity = makeIdentity() + const recipientIdentity = makeIdentity() + const { nostr, triggerEvent } = makeMockNostr() + const client = new LnbitsClient({ + serverPubkey: serverIdentity.publicKey, + relays: ['ws://test/'], + }) + client.initialize(nostr, recipientIdentity) + return { client, serverIdentity, recipientIdentity, triggerEvent } + } + + it('drops a forged event without resolving any pending RPC', () => { + const { client, serverIdentity, triggerEvent } = setupClient() + + // Pre-register a pending entry as `sendRpc` would have. + let resolveCalls = 0 + let rejectCalls = 0 + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(client as any).pending.set('req-forged', { + resolve: () => { + resolveCalls++ + }, + reject: () => { + rejectCalls++ + }, + timer: setTimeout(() => {}, 60_000), + }) + + // Forge a kind-21000 event: signed by attacker, pubkey overwritten + // to the server's. Content is bogus — the guard runs BEFORE + // decryptContentV2, so the ciphertext doesn't have to be valid. + const attackerKey = generateSecretKey() + const forged = forgeEvent( + { + kind: KIND_LNBITS_RPC, + content: 'irrelevant-because-guard-rejects-first', + tags: [], + created_at: Math.floor(Date.now() / 1000), + }, + serverIdentity.publicKey, + attackerKey, + ) + + triggerEvent(forged) + + expect(resolveCalls).toBe(0) + expect(rejectCalls).toBe(0) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((client as any).pending.has('req-forged')).toBe(true) + }) + + it('processes a legitimate server-signed reply (positive sanity)', () => { + const { client, serverIdentity, recipientIdentity, triggerEvent } = + setupClient() + + let resolved: unknown = null + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(client as any).pending.set('req-ok', { + resolve: (r: unknown) => { + resolved = r + }, + reject: () => {}, + timer: setTimeout(() => {}, 60_000), + }) + + // Encrypt a real LnbitsRpcResponse from server to recipient. + const responsePayload = JSON.stringify({ + status: 'OK', + request_id: 'req-ok', + data: { balanceSats: 12345 }, + }) + const ciphertext = encryptContentV2( + serverIdentity, + recipientIdentity.publicKey, + responsePayload, + ) + + const reply = finalizeEvent( + { + kind: KIND_LNBITS_RPC, + content: ciphertext, + tags: [['p', recipientIdentity.publicKey]], + created_at: Math.floor(Date.now() / 1000), + }, + serverIdentity.privateKey, + ) + + triggerEvent(reply) + + expect(resolved).toMatchObject({ + status: 'OK', + request_id: 'req-ok', + data: { balanceSats: 12345 }, + }) + // Note: `handleReply` itself doesn't delete the pending entry — the + // delete happens inside the real `sendRpc`-installed resolve closure. + // Our test installs its own resolve, so the entry remains; that's + // fine, we only need to assert resolve fired with the right payload. + }) +}) diff --git a/packages/lnbits/src/client.ts b/packages/lnbits/src/client.ts index 0d4fc51..ae40019 100644 --- a/packages/lnbits/src/client.ts +++ b/packages/lnbits/src/client.ts @@ -71,6 +71,10 @@ const LNBITS_KIND_RPC = 21000 * across server inbound, lnbits-bunker-client inbound, and ATM-client * inbound — see `aiolabs/lamassu-next#49` and the cross-codebase * coordination memory. + * + * @internal Exported for tests only. Production callers should go + * through `LnbitsClient.handleReply`, which already invokes + * this guard before decrypting. */ export function isAuthenticServerEvent( ev: NostrEvent,