From a2fd116f8147d209c758c299e9122450854224a4 Mon Sep 17 00:00:00 2001
From: Alex Gleason <alex@alexgleason.me>
Date: Wed, 19 Aug 2026 14:55:22 -0500
Subject: [PATCH] fix(nip46): require a secret in nostrconnect:// URIs

fromURI() matched the bunker's response against
`uri.searchParams.get('secret')`, which is `null` when the URI carries no
secret parameter. A bunker answering `{"result": null}` then satisfies
`response.result === null` and is adopted as the client's signer.

Since the client subscribes to the relays named in the URI and the
connection event is public, any participant on those relays can win that
race and sign on the user's behalf.

The secret is the only thing distinguishing the bunker we asked from
everyone else who saw the URI, so treat a missing or empty one as a
programming error and reject before subscribing. createNostrConnectURI()
already always sets it; this only affects URIs built elsewhere.
---
 nip46.test.ts | 27 +++++++++++++++++++++++++++
 nip46.ts      | 15 ++++++++++++---
 2 files changed, 39 insertions(+), 3 deletions(-)
 create mode 100644 nip46.test.ts

diff --git a/nip46.test.ts b/nip46.test.ts
new file mode 100644
index 0000000..cfb0fa9
--- /dev/null
+++ b/nip46.test.ts
@@ -0,0 +1,27 @@
+import { test, expect } from 'bun:test'
+
+import { BunkerSigner, createNostrConnectURI } from './nip46.ts'
+import { generateSecretKey, getPublicKey } from './pure.ts'
+
+const clientSecretKey = generateSecretKey()
+const clientPubkey = getPublicKey(clientSecretKey)
+
+test('createNostrConnectURI always includes the secret', () => {
+  const uri = new URL(createNostrConnectURI({ clientPubkey, relays: ['wss://relay.example.com'], secret: 'hunter2' }))
+
+  expect(uri.searchParams.get('secret')).toEqual('hunter2')
+})
+
+test('fromURI rejects a URI without a secret', async () => {
+  const uri = `nostrconnect://${clientPubkey}?relay=wss://relay.example.com`
+
+  // otherwise a bunker replying `{"result": null}` would match `get('secret')`
+  // and become the signer for this client
+  await expect(BunkerSigner.fromURI(clientSecretKey, uri)).rejects.toThrow(/no secret/)
+})
+
+test('fromURI rejects a URI with an empty secret', async () => {
+  const uri = `nostrconnect://${clientPubkey}?relay=wss://relay.example.com&secret=`
+
+  await expect(BunkerSigner.fromURI(clientSecretKey, uri)).rejects.toThrow(/no secret/)
+})
diff --git a/nip46.ts b/nip46.ts
index 664e8ed..5dc3015 100644
--- a/nip46.ts
+++ b/nip46.ts
@@ -192,8 +192,17 @@ export class BunkerSigner implements Signer {
     bunkerParams: BunkerSignerParams = {},
     maxWaitOrAbort: number | AbortSignal = 300_000,
   ): Promise<BunkerSigner> {
-    const signer = new BunkerSigner(clientSecretKey, bunkerParams)
     const uri = new URL(connectionURI)
+
+    // the secret is what tells the bunker we asked for apart from anyone else who
+    // saw the URI on the relay. without it there is nothing to compare the response
+    // against, and any pubkey that answers would be accepted as our signer.
+    const secret = uri.searchParams.get('secret')
+    if (!secret) {
+      throw new Error('nostrconnect:// URI has no secret')
+    }
+
+    const signer = new BunkerSigner(clientSecretKey, bunkerParams)
     const clientPubkey = getPublicKey(clientSecretKey)
 
     return new Promise((resolve, reject) => {
@@ -213,13 +222,13 @@ export class BunkerSigner implements Signer {
 
               const response = JSON.parse(decryptedContent)
 
-              if (response.result === uri.searchParams.get('secret')) {
+              if (response.result === secret) {
                 sub.close()
 
                 signer.bp = {
                   pubkey: event.pubkey,
                   relays: uri.searchParams.getAll('relay'),
-                  secret: uri.searchParams.get('secret'),
+                  secret,
                 }
                 signer.conversationKey = getConversationKey(clientSecretKey, event.pubkey)
                 signer.setupSubscription()