fix(relay,desktop): only advertise NIP-43 when enforced; probe pairing by supported_nips

crates/sprout-relay/src/nip11.rs

@@ -4,10 +4,20 @@ use serde::{Deserialize, Serialize};
 
 use crate::connection::MAX_FRAME_BYTES;
 
-/// NIPs supported by this relay, advertised in the NIP-11 document.
-/// Kept as a module-level constant so tests can verify it without constructing
-/// a full `Config` (which reads env vars and races with config.rs tests).
-pub(crate) const SUPPORTED_NIPS: &[u32] = &[1, 2, 10, 11, 16, 17, 23, 25, 29, 33, 38, 42, 43, 50];
+/// NIPs unconditionally supported by this relay, advertised in the NIP-11
+/// document. Kept as a module-level constant so tests can verify it without
+/// constructing a full `Config` (which reads env vars and races with
+/// config.rs tests).
+///
+/// NIP-43 (relay membership) is advertised separately by [`RelayInfo::build`]
+/// only when membership enforcement is actually enabled — see that function.
+pub(crate) const SUPPORTED_NIPS: &[u32] = &[1, 2, 10, 11, 16, 17, 23, 25, 29, 33, 38, 42, 50];
+
+/// NIP-43 (relay membership). Advertised only when the relay actually
+/// enforces membership (`SPROUT_REQUIRE_RELAY_MEMBERSHIP=true`) AND has a
+/// stable signing key — both are required for kind 13534/8000/8001 events
+/// to be verifiable by clients.
+pub(crate) const NIP_RELAY_MEMBERSHIP: u32 = 43;
 
 /// Relay information document served at `GET /` with `Accept: application/nostr+json`.
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -80,19 +90,39 @@ fn relay_limitation() -> RelayLimitation {
 impl RelayInfo {
     /// Builds the relay's NIP-11 information document.
     ///
-    /// `relay_pubkey` is the relay's own signing pubkey (hex), advertised as the
-    /// NIP-11 `self` field for NIP-43 membership verification.
-    pub fn build(relay_pubkey: Option<&str>) -> Self {
+    /// `relay_self` is the relay's own signing pubkey (hex), advertised as the
+    /// NIP-11 `self` field. NIP-11 defines `self` generically as the relay's
+    /// identity key; other NIPs reference it. Notably NIP-29 (group metadata
+    /// kinds 39000/39001/39002, which Sprout signs with `state.relay_keypair`
+    /// unconditionally) requires clients to verify those events against
+    /// `self`. Pass `Some` whenever the relay has a stable signing key.
+    ///
+    /// `advertise_nip43` controls whether NIP-43 (relay membership) is added
+    /// to `supported_nips`. Set `true` only when the relay actually emits and
+    /// gates on NIP-43 events — i.e. has a stable key AND enforces
+    /// membership. NIP-43 events are verified against `self`, so it is a
+    /// programmer error to advertise NIP-43 without a `relay_self`.
+    pub fn build(relay_self: Option<&str>, advertise_nip43: bool) -> Self {
+        debug_assert!(
+            !advertise_nip43 || relay_self.is_some(),
+            "advertise_nip43=true requires relay_self=Some — NIP-43 events are verified against `self`"
+        );
+
+        let mut supported_nips = SUPPORTED_NIPS.to_vec();
+        if advertise_nip43 {
+            supported_nips.push(NIP_RELAY_MEMBERSHIP);
+        }
+
         Self {
             name: "Sprout Relay".to_string(),
             description: "Sprout — private team communication relay".to_string(),
             pubkey: None,
             contact: None,
-            supported_nips: SUPPORTED_NIPS.to_vec(),
+            supported_nips,
             software: "https://github.com/sprout-rs/sprout".to_string(),
             version: env!("CARGO_PKG_VERSION").to_string(),
             limitation: Some(relay_limitation()),
-            relay_self: relay_pubkey.map(|s| s.to_string()),
+            relay_self: relay_self.map(|s| s.to_string()),
         }
     }
 }
@@ -101,14 +131,27 @@ impl RelayInfo {
 pub async fn relay_info_handler(
     axum::extract::State(state): axum::extract::State<std::sync::Arc<crate::state::AppState>>,
 ) -> axum::response::Json<RelayInfo> {
-    // Only advertise the NIP-11 `self` field when a stable relay key is configured.
-    // Ephemeral (auto-generated) keys change on restart, making signed events unverifiable.
-    let relay_pubkey = if state.config.relay_private_key.is_some() {
-        Some(state.relay_keypair.public_key().to_hex())
-    } else {
-        None
-    };
-    axum::response::Json(RelayInfo::build(relay_pubkey.as_deref()))
+    let (relay_self, advertise_nip43) = nip11_facts(&state);
+    axum::response::Json(RelayInfo::build(relay_self.as_deref(), advertise_nip43))
+}
+
+/// Derives the two NIP-11 facts that depend on runtime config:
+///
+/// - `relay_self`: the NIP-11 `self` pubkey, set whenever the relay has a
+///   stable signing key. Consumed by NIP-29 (group metadata verification)
+///   and NIP-43, among others. Ephemeral keys are excluded because they
+///   change on restart, leaving previously-signed events unverifiable.
+/// - `advertise_nip43`: whether to list NIP-43 in `supported_nips`. True
+///   only when membership is actually enforced AND we have a stable key
+///   (NIP-43 events must be verifiable against `self`).
+///
+/// Centralised so the content-negotiated root handler and the dedicated
+/// `/info` endpoint can't drift apart.
+pub(crate) fn nip11_facts(state: &crate::state::AppState) -> (Option<String>, bool) {
+    let has_stable_key = state.config.relay_private_key.is_some();
+    let relay_self = has_stable_key.then(|| state.relay_keypair.public_key().to_hex());
+    let advertise_nip43 = has_stable_key && state.config.require_relay_membership;
+    (relay_self, advertise_nip43)
 }
 
 #[cfg(test)]
@@ -140,9 +183,8 @@ mod tests {
     #[test]
     fn auth_required_is_advertised_true() {
         // REQ, EVENT, and COUNT all unconditionally require
-        // `AuthState::Authenticated` (see `crates/sprout-relay/src/handlers/`).
-        // The NIP-11 doc must reflect that or clients (e.g. the desktop pair
-        // flow) misroute unauthenticated peers.
+        // `AuthState::Authenticated` (see `crates/sprout-relay/src/handlers/`),
+        // so the NIP-11 doc must advertise it.
         assert!(relay_limitation().auth_required);
     }
 
@@ -156,4 +198,55 @@ mod tests {
             "supported_nips should be sorted"
         );
     }
+
+    #[test]
+    fn nip43_not_in_static_supported_nips() {
+        // NIP-43 advertisement is conditional on runtime config (stable signing
+        // key + membership enforcement) and must NOT live in the static list.
+        // The desktop pairing probe keys off this NIP — advertising it on
+        // open relays misroutes pairing peers to a non-existent /pair sidecar.
+        assert!(
+            !SUPPORTED_NIPS.contains(&NIP_RELAY_MEMBERSHIP),
+            "NIP-43 must be advertised only when advertise_nip43=true is passed to RelayInfo::build"
+        );
+    }
+
+    /// Open relay, ephemeral key — both `self` and NIP-43 are absent.
+    #[test]
+    fn build_open_relay_ephemeral_key_omits_self_and_nip43() {
+        let info = RelayInfo::build(None, false);
+        assert!(info.relay_self.is_none());
+        assert!(!info.supported_nips.contains(&NIP_RELAY_MEMBERSHIP));
+    }
+
+    /// Open relay with a stable signing key (e.g. for NIP-29 group metadata
+    /// signing): `self` MUST be advertised so clients can verify those
+    /// events; NIP-43 must NOT be, because the relay isn't enforcing
+    /// membership. This is the staging-default shape — the bug we're
+    /// fixing — and the regression we must not reintroduce.
+    #[test]
+    fn build_open_relay_stable_key_advertises_self_but_not_nip43() {
+        let pk = "0000000000000000000000000000000000000000000000000000000000000001";
+        let info = RelayInfo::build(Some(pk), false);
+        assert_eq!(info.relay_self.as_deref(), Some(pk));
+        assert!(!info.supported_nips.contains(&NIP_RELAY_MEMBERSHIP));
+    }
+
+    /// Membership-enforcing relay: both `self` and NIP-43 advertised.
+    #[test]
+    fn build_membership_relay_advertises_self_and_nip43() {
+        let pk = "0000000000000000000000000000000000000000000000000000000000000001";
+        let info = RelayInfo::build(Some(pk), true);
+        assert_eq!(info.relay_self.as_deref(), Some(pk));
+        assert!(info.supported_nips.contains(&NIP_RELAY_MEMBERSHIP));
+    }
+
+    /// NIP-43 events are verified against `self`; advertising NIP-43 without
+    /// `self` would give clients no way to verify membership events. The
+    /// debug_assert in `build` catches this in tests/debug builds.
+    #[test]
+    #[should_panic(expected = "advertise_nip43=true requires relay_self=Some")]
+    fn build_nip43_without_self_panics_in_debug() {
+        let _ = RelayInfo::build(None, true);
+    }
 }

crates/sprout-relay/src/router.rs

@@ -21,7 +21,7 @@ use crate::api;
 use crate::audio;
 use crate::connection::handle_connection;
 use crate::metrics::track_metrics;
-use crate::nip11::{relay_info_handler, RelayInfo};
+use crate::nip11::{nip11_facts, relay_info_handler, RelayInfo};
 use crate::state::AppState;
 
 /// Build the axum [`Router`] with all relay routes, middleware, and CORS configuration.
@@ -153,14 +153,10 @@ async fn nip11_or_ws_handler(
         .and_then(|v| v.to_str().ok())
         .unwrap_or("");
 
-    let relay_pubkey = if state.config.relay_private_key.is_some() {
-        Some(state.relay_keypair.public_key().to_hex())
-    } else {
-        None
-    };
+    let (relay_self, advertise_nip43) = nip11_facts(&state);
 
     if accept.contains("application/nostr+json") {
-        let info = RelayInfo::build(relay_pubkey.as_deref());
+        let info = RelayInfo::build(relay_self.as_deref(), advertise_nip43);
         return Json(info).into_response();
     }
 
@@ -179,7 +175,7 @@ async fn nip11_or_ws_handler(
                 }
             }
             // Not a WS request and not asking for nostr+json — serve NIP-11 as fallback.
-            let info = RelayInfo::build(relay_pubkey.as_deref());
+            let info = RelayInfo::build(relay_self.as_deref(), advertise_nip43);
             Json(info).into_response()
         }
     }

desktop/src-tauri/src/commands/pairing.rs

@@ -89,9 +89,13 @@ pub async fn start_pairing(
     let ws_url = relay_ws_url_with_override(&state);
     let http_url = relay_api_base_url_with_override(&state);
 
-    // Detect NIP-43: if the relay requires auth, the target device should
-    // connect to the /pair sidecar instead of the main relay.
-    let qr_relay_url = if probe_relay_requires_auth(&ws_url).await {
+    // NIP-43 relays gate connections on membership, so an unpaired peer can't
+    // reach the main relay yet — it must go through the /pair sidecar. Open
+    // relays (no NIP-43) accept the peer directly. We key off the relay's
+    // own NIP-11 declaration of NIP-43 support rather than `auth_required`,
+    // which is also true for plain NIP-42 / NIP-OA relays where the main
+    // relay is reachable.
+    let qr_relay_url = if probe_relay_supports_nip43(&ws_url).await {
         let mut url = url::Url::parse(&ws_url).map_err(|e| format!("invalid relay URL: {e}"))?;
         let path = url.path().trim_end_matches('/').to_string();
         url.set_path(&format!("{path}/pair"));
@@ -422,13 +426,20 @@ fn parse_relay_event(text: &str, sub_id: &str) -> Option<nostr_compat::Event> {
     serde_json::from_value(arr[2].clone()).ok()
 }
 
-/// Check the relay's NIP-11 information document to determine if auth is
-/// required (indicating NIP-43 access control). Returns `true` if the relay
-/// advertises `limitation.auth_required: true`, `false` otherwise.
+/// Check the relay's NIP-11 document to determine whether it advertises
+/// NIP-43 (relay membership). Returns `true` only if NIP-43 appears in the
+/// relay's `supported_nips`. Unreachable relays, malformed responses, and
+/// non-`ws(s)://` URLs all return `false`: we'd rather fail loudly against
+/// the main relay than misroute pairing to an undeployed `/pair` sidecar.
 ///
 /// Converts the WebSocket URL to HTTP(S) and fetches `GET /` with
 /// `Accept: application/nostr+json` per NIP-11.
-async fn probe_relay_requires_auth(relay_url: &str) -> bool {
+///
+/// We test for NIP-43 specifically rather than the broader
+/// `limitation.auth_required` flag because the latter is also set on plain
+/// NIP-42 / NIP-OA relays, which accept unpaired peers on the main relay
+/// and have no `/pair` sidecar.
+async fn probe_relay_supports_nip43(relay_url: &str) -> bool {
     // Convert ws(s):// to http(s):// for the NIP-11 fetch.
     let http_url = if let Some(rest) = relay_url.strip_prefix("wss://") {
         format!("https://{rest}")
@@ -458,10 +469,9 @@ async fn probe_relay_requires_auth(relay_url: &str) -> bool {
         Err(_) => return false,
     };
 
-    // Check limitation.auth_required per NIP-11.
-    json.get("limitation")
-        .and_then(|l| l.get("auth_required"))
-        .and_then(|v| v.as_bool())
+    json.get("supported_nips")
+        .and_then(|v| v.as_array())
+        .map(|arr| arr.iter().any(|n| n.as_u64() == Some(43)))
         .unwrap_or(false)
 }