diff --git a/src/signer/nip46_client.rs b/src/signer/nip46_client.rs index 40cd324..db405a3 100644 --- a/src/signer/nip46_client.rs +++ b/src/signer/nip46_client.rs @@ -613,15 +613,21 @@ impl Nip46ClientSigner { eprintln!("[nip46 pairing] could not derive a conversation with this author"); continue; }; - let Ok(plain) = nip44_decrypt(&conv, &event.content) else { - eprintln!("[nip46 pairing] payload did not decrypt (wrong conversation key)"); - continue; + let plain = match nip44_decrypt(&conv, &event.content) { + Ok(p) => p, + Err(e) => { + // Surface the REAL failure (HMAC vs wrong key vs invalid + // padding): a malformed NIP-44 frame looks identical to a + // wrong conversation key otherwise. + eprintln!( + "[nip46 pairing] payload did not decrypt: {e} (frame {} chars b64)", + event.content.len() + ); + continue; + } }; let Ok(request) = serde_json::from_str::(&plain) else { - eprintln!( - "[nip46 pairing] decrypted payload is not a NIP-46 request (len {})", - plain.len() - ); + eprintln!("[nip46 pairing] decrypted payload is not a NIP-46 request: {plain}"); continue; }; if request.method != "connect" { @@ -1855,32 +1861,70 @@ impl Signer for Nip46ClientSigner { } } -/// Minimal decrypted NIP-46 request. -#[derive(Debug, Deserialize)] +/// Minimal decrypted NIP-46 request. Deserialization is maximally lenient: +/// real signers deviate from the spec'd shape (`id` numeric, `params` an +/// object instead of an array, raw JSON grants embedded in params), and a +/// strict parse silently dropped the pairing handshake. Every scalar is +/// coerced to text instead of rejecting the request. +#[derive(Debug)] struct RawRequest { id: String, method: String, - #[serde(default, deserialize_with = "lenient_params")] params: Vec, } -/// NIP-46 params are conventionally strings, but real signers embed raw JSON -/// objects (Amber sends its requested_perms grant object as `connect` -/// params[1]). A strict `Vec` would reject the entire request and -/// silently drop the handshake, so non-string params are coerced to their -/// JSON text instead. -fn lenient_params<'de, D>(deserializer: D) -> Result, D::Error> -where - D: serde::Deserializer<'de>, -{ - let values: Vec = Vec::deserialize(deserializer)?; - Ok(values - .into_iter() - .map(|v| match v { - serde_json::Value::String(s) => s, - other => other.to_string(), - }) - .collect()) +impl<'de> serde::Deserialize<'de> for RawRequest { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + fn to_text(v: &serde_json::Value) -> String { + match v { + serde_json::Value::String(s) => s.clone(), + other => other.to_string(), + } + } + let value = serde_json::Value::deserialize(deserializer)?; + let mut obj = match value { + serde_json::Value::Object(map) => map, + // Double-encoded payload: a JSON string wrapping the request + // object. Unwrap one level and retry. + serde_json::Value::String(ref s) => { + match serde_json::from_str::(s) { + Ok(serde_json::Value::Object(map)) => map, + _ => { + return Ok(RawRequest { + id: String::new(), + method: String::new(), + params: vec![s.clone()], + }); + } + } + } + // Any other non-object is not a JSON-RPC request; surface it as + // an empty method (the caller logs and ignores it) rather than + // failing, so the raw text still reaches the diagnostic log. + other => { + return Ok(RawRequest { + id: String::new(), + method: String::new(), + params: vec![to_text(&other)], + }); + } + }; + let id = obj.remove("id").map(|v| to_text(&v)).unwrap_or_default(); + let method = obj + .remove("method") + .map(|v| to_text(&v)) + .unwrap_or_default(); + let params = match obj.remove("params") { + Some(serde_json::Value::Array(arr)) => arr.iter().map(to_text).collect(), + Some(serde_json::Value::Null) | None => Vec::new(), + // `params` shaped as an object: treat it as a single param. + Some(other) => vec![to_text(&other)], + }; + Ok(RawRequest { id, method, params }) + } } fn response_ok(id: &str, result: String) -> String { @@ -1976,6 +2020,64 @@ mod raw_request_tests { ); } + #[test] + fn coerces_numeric_ids() { + // Some signers use numeric request ids; a strict String id used to + // reject the whole request and silently drop the handshake. + let req: RawRequest = + serde_json::from_str(r#"{"id":42,"method":"connect","params":[]}"#).unwrap(); + assert_eq!(req.id, "42"); + let req: RawRequest = + serde_json::from_str(r#"{"id":1789267093,"method":"connect","params":["aa","bb"]}"#) + .unwrap(); + assert_eq!(req.id, "1789267093"); + assert_eq!(req.method, "connect"); + } + + #[test] + fn missing_id_defaults_to_empty() { + let req: RawRequest = serde_json::from_str(r#"{"method":"connect"}"#).unwrap(); + assert_eq!(req.id, ""); + assert_eq!(req.method, "connect"); + } + + #[test] + fn unwraps_double_encoded_request() { + // A signer that JSON-stringifies the whole request object. + let inner = r#"{"id":"9","method":"connect","params":["aa"]}"#; + let raw = serde_json::Value::String(inner.to_string()).to_string(); + let req: RawRequest = serde_json::from_str(&raw).expect("must parse"); + assert_eq!(req.id, "9"); + assert_eq!(req.method, "connect"); + assert_eq!(req.params, vec!["aa".to_string()]); + } + + #[test] + fn accepts_object_shaped_params() { + // `params` given as a bare object rather than an array. + let req: RawRequest = + serde_json::from_str(r#"{"id":"x","method":"connect","params":{"sign_event":[0]}}"#) + .expect("must parse"); + assert_eq!(req.params.len(), 1); + assert!(req.params[0].starts_with('{')); + } + + #[test] + fn never_fails_on_valid_json() { + // Whatever a signer sends that is valid JSON must deserialize; the + // worst case is an empty method the caller logs and ignores. + for raw in [ + r#"[1,2,3]"#, + r#""just a string""#, + r#"42"#, + r#"null"#, + r#"{"no":"fields"}"#, + ] { + let req: RawRequest = serde_json::from_str(raw).expect("must not fail"); + assert!(raw.contains("fields") || req.method.is_empty() || req.method == "connect"); + } + } + #[test] fn parses_string_params_unchanged() { let req: RawRequest =