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

@@ -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/)
+})

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()