diff --git a/src/signer/nip46_client.rs b/src/signer/nip46_client.rs index d7e356a..9313caf 100644 --- a/src/signer/nip46_client.rs +++ b/src/signer/nip46_client.rs @@ -683,6 +683,44 @@ impl Nip46ClientSigner { continue; } }; + let shaped: serde_json::Value = + serde_json::from_str(&plain).unwrap_or(serde_json::Value::Null); + // Per NIP-46, for a client-initiated `nostrconnect://` the signer + // sends a connect *response* — `{"id":…,"result":""}` — + // not a `connect` request: there is nothing to answer, the secret + // echo IS the handshake. (Spec: "the _remote-signer_ … then sends + // `connect` *response* event to the `client-pubkey`"; result is + // `"ack"` OR the secret.) The legacy request shape is still + // handled below for signers that send `{"method":"connect"}`. + if shaped.get("method").is_none() + && (shaped.get("result").is_some() || shaped.get("error").is_some()) + { + if shaped.get("error").is_some() && !shaped["error"].is_null() { + let msg = shaped["error"] + .as_str() + .map(|s| s.to_string()) + .unwrap_or_else(|| shaped["error"].to_string()); + return Err(format!("The signer rejected the connection: {msg}")); + } + let result = shaped["result"].as_str().unwrap_or(""); + if result != secret && result != "ack" { + eprintln!( + "[nip46 pairing] connect response echoed the wrong secret — ignoring" + ); + if live_capture { + pairing_trace("connect response echoed the wrong secret — ignoring"); + } + continue; + } + eprintln!("[nip46 pairing] connect response accepted (secret echo verified)"); + if live_capture { + pairing_trace("connect response accepted (secret echo verified)"); + } + // Tear down the pairing subscription BEFORE handing over: + // from here on the demux loop owns the conversation. + client.unsubscribe(subscription.id()).await.ok(); + break Ok::<_, String>((event.pubkey, conv, None)); + } let Ok(request) = serde_json::from_str::(&plain) else { eprintln!("[nip46 pairing] decrypted payload is not a NIP-46 request: {plain}"); if live_capture { @@ -700,8 +738,8 @@ impl Nip46ClientSigner { } continue; } - // NIP-46: the client answers the signer's `connect` with the - // secret as the result; the signer verifies the echo. + // Legacy path: a signer that sends an actual `connect` *request* + // gets the secret as the answer, and it verifies the echo. let response = response_ok(&request.id, secret.clone()); // Tear down the pairing subscription BEFORE answering: from here // on the demux loop owns the conversation. If both subscriptions diff --git a/tests/nip46_e2e.rs b/tests/nip46_e2e.rs index 90d50c6..470b7e7 100644 --- a/tests/nip46_e2e.rs +++ b/tests/nip46_e2e.rs @@ -476,6 +476,7 @@ async fn run_fake_scanner( client_pk: PublicKey, expected_secret: String, identity: Keys, + connect_shape: &'static str, ) { let (mut ws, _) = tokio_tungstenite::connect_async(&relay_url) .await @@ -491,12 +492,21 @@ async fn run_fake_scanner( let conversation = ConversationKey::derive(identity.secret_key(), &client_pk).unwrap(); // The scanned URI tells us who to contact and what secret to echo. - let connect_req = json!({ - "id": "pair-1", - "method": "connect", - "params": [expected_secret.clone()], - }); - let content = nip44_enc(&conversation, &connect_req.to_string()); + // Two shapes: + // - "request": {"id","method":"connect","params":[secret]} — what the + // e2e originally simulated; the client answers with the secret. + // - "response": {"id","result":secret} — what NIP-46 actually specifies + // for nostrconnect:// ("the _remote-signer_ … sends `connect` + // *response* event"), and what Amber sends. No answer expected. + let connect_msg = match connect_shape { + "response" => json!({ "id": "pair-1", "result": expected_secret.clone() }), + _ => json!({ + "id": "pair-1", + "method": "connect", + "params": [expected_secret.clone()], + }), + }; + let content = nip44_enc(&conversation, &connect_msg.to_string()); let out = EventBuilder::new(Kind::NostrConnect, content) .tags([Tag::parse(["p", client_pk.to_hex().as_str()]).unwrap()]) .finalize(&identity) @@ -602,12 +612,28 @@ async fn run_fake_scanner( #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn nip46_qr_pairing_handshake_and_sign() { + run_qr_pairing("request").await; +} + +/// The shape NIP-46 actually specifies for a `nostrconnect://` scan — and +/// what Amber sends — is a connect *response* (`{"id","result":""}`), +/// not a `connect` request. Pairing must complete on that shape too. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn nip46_qr_pairing_connect_response_shape() { + run_qr_pairing("response").await; +} + +async fn run_qr_pairing(connect_shape: &'static str) { let relay_url = start_relay().await; // Isolated vault + pairing relay, serialized with the other e2e test. let app = { let _guard = VAULT_ENV_LOCK.lock().unwrap(); - let tmp = std::env::temp_dir().join(format!("keynectr-e2e-pair-{}", std::process::id())); + let tmp = std::env::temp_dir().join(format!( + "keynectr-e2e-pair-{}-{}", + std::process::id(), + connect_shape + )); std::fs::create_dir_all(&tmp).unwrap(); std::fs::write( tmp.join("profiles_vault.json"), @@ -662,6 +688,7 @@ async fn nip46_qr_pairing_handshake_and_sign() { client_pk, secret.clone(), identity.clone(), + connect_shape, )); // Wait for scan -> secret echo -> identity adoption.