From 1712cbd2ed9c5742064239975dde4e3d9cd337a3 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 16 Jul 2026 23:01:52 +1000 Subject: [PATCH] fix(desktop): harden the relay-edit and repos-map concurrency edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the review findings on the lazy multi-workspace agent stack (review of f3baa96) — four hardening fixes, no behavior change on the happy path: - Serialize apply_workspace's .repos-dirs.json upsert on managed_agents_store_lock. A community relay edit fires rebind_agent_relay (which moves map entries under that lock) nearly simultaneously with the reinit apply, and both are read-modify-writes of the same file — unserialized, one side's update could be lost. - Write .repos-dirs.json via temp file + rename. A crash mid-write left truncated JSON, which read_repos_dir_map degrades to an empty map — silently sending spawns back to the shared REPOS fallback, exactly the cross-workspace hazard the map exists to prevent. - Pin the Rust/TS relay-URL normalizers together with a shared fixture (desktop/fixtures/relay-url-normalization.json) consumed by both relay tests and agentRelayScope.test.mjs, so an edit that lands on only one side fails the other side's tests instead of shipping a scoping skew. Both normalizers' doc comments now point at it. - Sequence updateCommunity's state commit (and the reinit it triggers) after the rebindAgentRelay IPC settles. Fire-and-forget let apply_workspace(newUrl) race ahead of the rebind: activation ran while start-on-launch agents were still pinned to the old URL and marked the relay activated for the session, silently skipping them until app relaunch. A rebind failure still commits — pins stay recoverable by re-editing the community. The fifth finding (stale "running in other communities" count) was already resolved by the slow cross-community poll tier added in 0f8fa7e. Split relay.rs's inline test module into relay/tests.rs (file-size guard, mirroring repos/tests.rs). Tested: cargo test --manifest-path desktop/src-tauri/Cargo.toml (1435 passed, incl. the new temp-file and fixture-agreement tests), clippy --all-targets -D warnings, rustfmt, desktop pnpm test (2955 passed, incl. the new fixture-agreement test), tsc --noEmit, biome check, pnpm check:file-sizes. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- desktop/fixtures/relay-url-normalization.json | 47 +++ desktop/src-tauri/src/commands/workspace.rs | 23 +- desktop/src-tauri/src/managed_agents/repos.rs | 11 +- .../src/managed_agents/repos/tests.rs | 24 ++ desktop/src-tauri/src/relay.rs | 348 +---------------- desktop/src-tauri/src/relay/tests.rs | 365 ++++++++++++++++++ .../features/agents/agentRelayScope.test.mjs | 28 +- .../src/features/agents/agentRelayScope.ts | 4 +- .../features/communities/useCommunities.tsx | 45 ++- 9 files changed, 527 insertions(+), 368 deletions(-) create mode 100644 desktop/fixtures/relay-url-normalization.json create mode 100644 desktop/src-tauri/src/relay/tests.rs diff --git a/desktop/fixtures/relay-url-normalization.json b/desktop/fixtures/relay-url-normalization.json new file mode 100644 index 000000000..f8789dbe0 --- /dev/null +++ b/desktop/fixtures/relay-url-normalization.json @@ -0,0 +1,47 @@ +[ + { + "input": "wss://relay.example.com", + "canonical": "wss://relay.example.com", + "note": "already canonical" + }, + { + "input": " wss://relay.example.com// ", + "canonical": "wss://relay.example.com", + "note": "surrounding whitespace and trailing slashes are cosmetic" + }, + { + "input": "WSS://Relay.Example.COM:3000/Path", + "canonical": "wss://relay.example.com:3000/Path", + "note": "scheme and authority are case-insensitive (RFC 3986); the path is not" + }, + { + "input": "WS://RELAY-A.example.com:3000/", + "canonical": "ws://relay-a.example.com:3000", + "note": "ws scheme and port survive; case and trailing slash fold away" + }, + { + "input": "wss://Relay.Example.com/Path?Query=Value", + "canonical": "wss://relay.example.com/Path?Query=Value", + "note": "path and query are preserved case-sensitively" + }, + { + "input": "wss://relay.example.com/path//", + "canonical": "wss://relay.example.com/path", + "note": "trailing slashes after a path are cosmetic too" + }, + { + "input": "not-a-url/", + "canonical": "not-a-url", + "note": "schemeless values pass through trim/slash cleanup only" + }, + { + "input": "", + "canonical": "", + "note": "empty stays empty" + }, + { + "input": " ", + "canonical": "", + "note": "whitespace-only trims to empty" + } +] diff --git a/desktop/src-tauri/src/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index 0a535b033..7368a700d 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -187,11 +187,24 @@ pub async fn apply_workspace( // symlink re-point below when a later switch moves it. Must land // before the activation task spawned after this closure starts // this workspace's agents. A bad candidate clears the entry, same - // as the dotfile above. - if let Err(error) = - persist_workspace_repos_dir(nest, &relay_url, effective_repos_dir.as_deref()) - { - eprintln!("buzz-desktop: persist per-relay repos dir failed: {error}"); + // as the dotfile above. Serialized on the store lock: a community + // relay edit fires rebind_agent_relay (which moves this map's + // entries under that lock) nearly simultaneously with this apply, + // and both are read-modify-writes of the same file — unserialized, + // one side's update would be lost. + match state.managed_agents_store_lock.lock() { + Ok(_store_guard) => { + if let Err(error) = persist_workspace_repos_dir( + nest, + &relay_url, + effective_repos_dir.as_deref(), + ) { + eprintln!("buzz-desktop: persist per-relay repos dir failed: {error}"); + } + } + Err(error) => { + eprintln!("buzz-desktop: persist per-relay repos dir skipped: {error}"); + } } if let Err(error) = ensure_repos_symlink(nest, effective_repos_dir.as_deref()) { eprintln!("buzz-desktop: repos dir setup failed: {error}"); diff --git a/desktop/src-tauri/src/managed_agents/repos.rs b/desktop/src-tauri/src/managed_agents/repos.rs index b7c6b8d53..3ad7d2c1c 100644 --- a/desktop/src-tauri/src/managed_agents/repos.rs +++ b/desktop/src-tauri/src/managed_agents/repos.rs @@ -273,6 +273,12 @@ fn read_repos_dir_map(nest_root: &Path) -> BTreeMap { /// Persist the per-relay `repos_dir` map, removing the file when empty /// (mirrors [`write_persisted_repos_dir`]'s clear-by-removal). +/// +/// The write goes through a temp file + rename so a crash mid-write cannot +/// leave truncated JSON behind: [`read_repos_dir_map`] degrades a malformed +/// file to an *empty* map, which would silently send every workspace's spawn +/// back to the shared `REPOS` fallback — exactly the cross-workspace hazard +/// the map exists to prevent. fn write_repos_dir_map(nest_root: &Path, map: &BTreeMap) -> Result<(), String> { let path = nest_root.join(REPOS_DIR_MAP_FILE); if map.is_empty() { @@ -284,7 +290,10 @@ fn write_repos_dir_map(nest_root: &Path, map: &BTreeMap) -> Resu } let json = serde_json::to_string_pretty(map) .map_err(|e| format!("serialize {REPOS_DIR_MAP_FILE}: {e}"))?; - fs::write(&path, format!("{json}\n")).map_err(|e| format!("write {}: {e}", path.display())) + let tmp = nest_root.join(format!("{REPOS_DIR_MAP_FILE}.tmp")); + fs::write(&tmp, format!("{json}\n")).map_err(|e| format!("write {}: {e}", tmp.display()))?; + fs::rename(&tmp, &path) + .map_err(|e| format!("rename {} -> {}: {e}", tmp.display(), path.display())) } /// Upsert (or clear, on `None`/empty) the applied workspace's `repos_dir` in diff --git a/desktop/src-tauri/src/managed_agents/repos/tests.rs b/desktop/src-tauri/src/managed_agents/repos/tests.rs index 67a93df54..ff8e2cfc9 100644 --- a/desktop/src-tauri/src/managed_agents/repos/tests.rs +++ b/desktop/src-tauri/src/managed_agents/repos/tests.rs @@ -498,6 +498,30 @@ fn repos_dir_map_rejects_empty_relay_and_survives_malformed_file() { ); } +#[test] +fn repos_dir_map_write_leaves_no_temp_file() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + + // The map is written via temp file + rename so a crash mid-write can + // never leave truncated JSON (read degrades malformed to an empty map, + // silently dropping isolation). A stale temp file — e.g. from a crash + // between write and rename — must be overwritten, and no temp file may + // outlive a successful write. + fs::write(root.join(".repos-dirs.json.tmp"), "half-written{").unwrap(); + persist_workspace_repos_dir(&root, "wss://relay-a.example", Some("/Users/me/DevA")).unwrap(); + + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://relay-a.example").as_deref(), + Some("/Users/me/DevA") + ); + assert!( + !root.join(".repos-dirs.json.tmp").exists(), + "the temp file is renamed into place, not left behind" + ); +} + #[test] fn rebind_workspace_repos_dir_moves_entry_to_new_relay() { let tmp = tempfile::tempdir().unwrap(); diff --git a/desktop/src-tauri/src/relay.rs b/desktop/src-tauri/src/relay.rs index f5652f124..16b30e10c 100644 --- a/desktop/src-tauri/src/relay.rs +++ b/desktop/src-tauri/src/relay.rs @@ -80,6 +80,12 @@ pub fn effective_agent_relay_url(record_relay: &str, workspace_relay: &str) -> S /// surrounding whitespace, trailing slashes, or scheme/host case (both are /// case-insensitive per RFC 3986). Any path or query is preserved /// case-sensitively. +/// +/// Must stay in lockstep with the frontend mirror +/// (`normalizeRelayUrlForCompare` in `desktop/src/features/agents/ +/// agentRelayScope.ts`) — the agreement is pinned by the shared fixture +/// `desktop/fixtures/relay-url-normalization.json`, consumed by both sides' +/// unit tests. Extend the fixture with any behavior change. pub fn normalize_relay_url(url: &str) -> String { let trimmed = url.trim().trim_end_matches('/'); match trimmed.split_once("://") { @@ -641,344 +647,4 @@ pub async fn submit_event_with_keys( // ── Tests ─────────────────────────────────────────────────────────────────── #[cfg(test)] -mod tests { - use super::{ - build_profile_event, classify_intercepted_response, effective_agent_relay_url, - normalize_relay_url, parse_command_response, relay_http_base_url, relay_urls_equivalent, - MALFORMED_RESPONSE_MESSAGE, - }; - use serde::Deserialize; - - // ── effective_agent_relay_url: per-agent override precedence ───────────── - - #[test] - fn explicit_relay_wins_over_workspace() { - // An explicit per-agent relay pins the agent there regardless of the - // active workspace — this is the override taking highest precedence. - assert_eq!( - effective_agent_relay_url("wss://relay.other.com", "wss://staging.example.com"), - "wss://relay.other.com" - ); - } - - #[test] - fn explicit_relay_wins_even_when_equal_to_workspace() { - // No special-casing when the pin happens to match the active workspace. - assert_eq!( - effective_agent_relay_url("wss://staging.example.com", "wss://staging.example.com"), - "wss://staging.example.com" - ); - } - - #[test] - fn empty_relay_falls_back_to_workspace() { - // A never-set record resolves to the active workspace relay at read-time, - // so a stale stored default can never make it load-bearing. - assert_eq!( - effective_agent_relay_url("", "wss://staging.example.com"), - "wss://staging.example.com" - ); - } - - #[test] - fn whitespace_only_relay_falls_back_to_workspace() { - // Whitespace-only is treated as unset, same as empty. - assert_eq!( - effective_agent_relay_url(" ", "wss://staging.example.com"), - "wss://staging.example.com" - ); - } - - // ── normalize_relay_url / relay_urls_equivalent ────────────────────────── - - #[test] - fn normalize_strips_whitespace_and_trailing_slashes() { - assert_eq!( - normalize_relay_url(" wss://relay.example.com// "), - "wss://relay.example.com" - ); - } - - #[test] - fn normalize_lowercases_scheme_and_host_only() { - // Scheme and authority are case-insensitive (RFC 3986); a path is not. - assert_eq!( - normalize_relay_url("WSS://Relay.Example.COM:3000/Path"), - "wss://relay.example.com:3000/Path" - ); - } - - #[test] - fn normalize_passes_through_schemeless_values() { - // Not a URL — nothing to case-fold beyond trim/slash cleanup. - assert_eq!(normalize_relay_url("not-a-url/"), "not-a-url"); - } - - #[test] - fn equivalence_ignores_cosmetic_differences() { - assert!(relay_urls_equivalent( - "wss://Relay.Example/", - " wss://relay.example" - )); - } - - #[test] - fn equivalence_distinguishes_real_differences() { - // Different host, port, or scheme = a different relay. - assert!(!relay_urls_equivalent( - "wss://relay-a.example", - "wss://relay-b.example" - )); - assert!(!relay_urls_equivalent( - "ws://relay.example:3000", - "ws://relay.example:3001" - )); - assert!(!relay_urls_equivalent( - "ws://relay.example", - "wss://relay.example" - )); - } - - // ── relay_http_base_url scheme conversion ──────────────────────────────── - - #[test] - fn loopback_ws_localhost_preserves_authority() { - // Tenant host-binding keys off the HTTP Host/authority. The desktop must - // not rewrite localhost to 127.0.0.1, or local dev HTTP calls target a - // different unmapped community than the WebSocket URL. - assert_eq!( - relay_http_base_url("ws://localhost:3000"), - "http://localhost:3000" - ); - } - - #[test] - fn loopback_trailing_slash_removed_authority_preserved() { - assert_eq!( - relay_http_base_url("ws://localhost:3000/"), - "http://localhost:3000" - ); - } - - #[test] - fn remote_wss_host_unchanged() { - assert_eq!( - relay_http_base_url("wss://relay.example.com"), - "https://relay.example.com" - ); - } - - #[test] - fn loopback_ipv4_literal_unchanged() { - assert_eq!( - relay_http_base_url("ws://127.0.0.1:3000"), - "http://127.0.0.1:3000" - ); - } - - #[test] - fn localhost_substring_host_unchanged() { - assert_eq!( - relay_http_base_url("ws://localhost.evil.com:3000"), - "http://localhost.evil.com:3000" - ); - } - - #[test] - fn loopback_wss_localhost_preserves_authority() { - assert_eq!( - relay_http_base_url("wss://localhost:3000"), - "https://localhost:3000" - ); - } - - // ── classify_intercepted_response ──────────────────────────────────────── - - #[test] - fn intercepted_cloudflare_host_returns_some() { - let result = classify_intercepted_response("sqprod.cloudflareaccess.com", "text/html"); - assert!(result.is_some()); - let msg = result.unwrap(); - assert!( - msg.starts_with("relay unreachable:"), - "should have unreachable prefix" - ); - assert!(msg.contains("Cloudflare"), "should mention Cloudflare"); - } - - #[test] - fn intercepted_cloudflare_apex_host_returns_some() { - // The apex domain itself should also match. - let result = classify_intercepted_response("cloudflareaccess.com", "application/json"); - assert!(result.is_some()); - let msg = result.unwrap(); - assert!(msg.starts_with("relay unreachable:")); - assert!(msg.contains("Cloudflare")); - } - - #[test] - fn intercepted_non_cloudflare_html_returns_some() { - let result = - classify_intercepted_response("proxy.corporate.example", "text/html; charset=utf-8"); - assert!(result.is_some()); - let msg = result.unwrap(); - assert!(msg.starts_with("relay unreachable:")); - } - - #[test] - fn normal_relay_json_returns_none() { - let result = classify_intercepted_response("relay.myapp.example.com", "application/json"); - assert!(result.is_none()); - } - - #[test] - fn content_type_case_insensitive() { - // Uppercase content-type must still be detected. - let result = classify_intercepted_response("proxy.example.com", "TEXT/HTML"); - assert!(result.is_some()); - assert!(result.unwrap().starts_with("relay unreachable:")); - } - - #[test] - fn evil_suffix_does_not_match_cloudflare() { - // A host whose suffix happens to contain the Cloudflare string but is - // not actually a subdomain must NOT match. - let result = classify_intercepted_response( - "notcloudflareaccess.com.evil.example", - "application/json", - ); - assert!( - result.is_none(), - "false suffix match should not trigger Cloudflare branch" - ); - } - - // classify_request_error requires a real reqwest::Error (not publicly - // constructable) — tested indirectly through integration; skipped here. - - // ── parse_json_response malformed-body contract ────────────────────────── - - #[test] - fn malformed_response_message_stays_off_unreachable_bucket() { - // A reached-but-malformed 2xx body is not a connectivity failure. If this - // message ever regains the "relay unreachable:" prefix, the frontend - // classifier would misroute it as unreachable — pin that it never does. - assert!( - !MALFORMED_RESPONSE_MESSAGE.starts_with("relay unreachable:"), - "malformed-response message must not match the unreachable prefix" - ); - } - - // ── parse_command_response ─────────────────────────────────────────────── - - #[derive(Debug, Deserialize, PartialEq)] - struct ChannelCreated { - channel_id: String, - } - - #[test] - fn parse_command_response_decodes_typed_payload() { - let msg = r#"response:{"channel_id":"abc123"}"#; - let parsed: ChannelCreated = parse_command_response(msg).expect("should parse"); - assert_eq!( - parsed, - ChannelCreated { - channel_id: "abc123".to_string() - } - ); - } - - #[test] - fn parse_command_response_accepts_raw_json_fallback() { - // Backward-compat: relays that emit raw JSON (no prefix) still work. - let msg = r#"{"channel_id":"abc"}"#; - let parsed: ChannelCreated = parse_command_response(msg).expect("fallback parse"); - assert_eq!( - parsed, - ChannelCreated { - channel_id: "abc".to_string() - } - ); - } - - #[test] - fn parse_command_response_rejects_invalid_prefixed_json() { - let msg = "response:not-json"; - let result: Result = parse_command_response(msg); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("response parse failed")); - } - - #[test] - fn parse_command_response_rejects_garbage() { - let msg = "totally not json or response"; - let result: Result = parse_command_response(msg); - assert!(result.is_err()); - } - - // ── build_profile_event ────────────────────────────────────────────────── - - /// Generate a valid NIP-OA auth tag JSON string signed by a fresh owner key - /// and addressed to `agent_keys`. - /// - /// Uses `nostr_compat` (nostr 0.36) for the owner keys because - /// `buzz_sdk_pkg::nip_oa::compute_auth_tag` expects nostr 0.36 types. - /// The agent pubkey is bridged via hex encoding. - fn make_valid_auth_tag(agent_keys: &nostr::Keys) -> String { - let owner_keys = nostr::Keys::generate(); - let agent_pubkey_hex = agent_keys.public_key().to_hex(); - let agent_compat_pubkey = - nostr::PublicKey::from_hex(&agent_pubkey_hex).expect("valid hex pubkey should parse"); - buzz_sdk_pkg::nip_oa::compute_auth_tag(&owner_keys, &agent_compat_pubkey, "") - .expect("compute_auth_tag should not fail with distinct keys") - } - - #[test] - fn profile_event_with_valid_auth_tag() { - let agent_keys = nostr::Keys::generate(); - let tag_json = make_valid_auth_tag(&agent_keys); - let event = build_profile_event(&agent_keys, "TestBot", None, Some(&tag_json)) - .expect("should succeed with a valid auth tag"); - - // Exactly one "auth" tag must be present. - let auth_tags: Vec<_> = event - .tags - .iter() - .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) - .collect(); - assert_eq!(auth_tags.len(), 1, "expected exactly 1 auth tag"); - - // Must be a kind:0 (Metadata) event. - assert_eq!(event.kind, nostr::Kind::Metadata); - } - - #[test] - fn profile_event_without_auth_tag() { - let agent_keys = nostr::Keys::generate(); - let event = build_profile_event(&agent_keys, "TestBot", None, None) - .expect("should succeed without an auth tag"); - - // No "auth" tags should be present. - let auth_tags: Vec<_> = event - .tags - .iter() - .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) - .collect(); - assert_eq!(auth_tags.len(), 0, "expected no auth tags"); - - assert_eq!(event.kind, nostr::Kind::Metadata); - } - - #[test] - fn profile_event_rejects_invalid_auth_tag() { - let agent_keys = nostr::Keys::generate(); - // Structurally valid JSON array but with a bogus signature — verification must fail. - let bad_json = format!(r#"["auth","{}","","{}"]"#, "a".repeat(64), "b".repeat(128)); - let result = build_profile_event(&agent_keys, "TestBot", None, Some(&bad_json)); - assert!(result.is_err(), "should reject an invalid auth tag"); - assert!( - result.unwrap_err().contains("verification failed"), - "error message should mention verification failure" - ); - } -} +mod tests; diff --git a/desktop/src-tauri/src/relay/tests.rs b/desktop/src-tauri/src/relay/tests.rs new file mode 100644 index 000000000..7690ecc0f --- /dev/null +++ b/desktop/src-tauri/src/relay/tests.rs @@ -0,0 +1,365 @@ +use super::{ + build_profile_event, classify_intercepted_response, effective_agent_relay_url, + normalize_relay_url, parse_command_response, relay_http_base_url, relay_urls_equivalent, + MALFORMED_RESPONSE_MESSAGE, +}; +use serde::Deserialize; + +// ── effective_agent_relay_url: per-agent override precedence ───────────── + +#[test] +fn explicit_relay_wins_over_workspace() { + // An explicit per-agent relay pins the agent there regardless of the + // active workspace — this is the override taking highest precedence. + assert_eq!( + effective_agent_relay_url("wss://relay.other.com", "wss://staging.example.com"), + "wss://relay.other.com" + ); +} + +#[test] +fn explicit_relay_wins_even_when_equal_to_workspace() { + // No special-casing when the pin happens to match the active workspace. + assert_eq!( + effective_agent_relay_url("wss://staging.example.com", "wss://staging.example.com"), + "wss://staging.example.com" + ); +} + +#[test] +fn empty_relay_falls_back_to_workspace() { + // A never-set record resolves to the active workspace relay at read-time, + // so a stale stored default can never make it load-bearing. + assert_eq!( + effective_agent_relay_url("", "wss://staging.example.com"), + "wss://staging.example.com" + ); +} + +#[test] +fn whitespace_only_relay_falls_back_to_workspace() { + // Whitespace-only is treated as unset, same as empty. + assert_eq!( + effective_agent_relay_url(" ", "wss://staging.example.com"), + "wss://staging.example.com" + ); +} + +// ── normalize_relay_url / relay_urls_equivalent ────────────────────────── + +/// One vector of the shared fixture consumed by BOTH this test and the +/// frontend mirror's (`agentRelayScope.test.mjs`): record pins are +/// stamped by `normalize_relay_url` and compared by the frontend's +/// `normalizeRelayUrlForCompare`, so an edit that lands on only one side +/// must fail the other side's tests instead of shipping a scoping skew. +#[derive(Deserialize)] +struct NormalizationVector { + input: String, + canonical: String, +} + +#[test] +fn normalize_agrees_with_shared_frontend_fixture() { + let vectors: Vec = serde_json::from_str(include_str!( + "../../../fixtures/relay-url-normalization.json" + )) + .unwrap(); + assert!(!vectors.is_empty(), "fixture must not be empty"); + for vector in &vectors { + assert_eq!( + normalize_relay_url(&vector.input), + vector.canonical, + "input: {:?}", + vector.input + ); + } +} + +#[test] +fn normalize_strips_whitespace_and_trailing_slashes() { + assert_eq!( + normalize_relay_url(" wss://relay.example.com// "), + "wss://relay.example.com" + ); +} + +#[test] +fn normalize_lowercases_scheme_and_host_only() { + // Scheme and authority are case-insensitive (RFC 3986); a path is not. + assert_eq!( + normalize_relay_url("WSS://Relay.Example.COM:3000/Path"), + "wss://relay.example.com:3000/Path" + ); +} + +#[test] +fn normalize_passes_through_schemeless_values() { + // Not a URL — nothing to case-fold beyond trim/slash cleanup. + assert_eq!(normalize_relay_url("not-a-url/"), "not-a-url"); +} + +#[test] +fn equivalence_ignores_cosmetic_differences() { + assert!(relay_urls_equivalent( + "wss://Relay.Example/", + " wss://relay.example" + )); +} + +#[test] +fn equivalence_distinguishes_real_differences() { + // Different host, port, or scheme = a different relay. + assert!(!relay_urls_equivalent( + "wss://relay-a.example", + "wss://relay-b.example" + )); + assert!(!relay_urls_equivalent( + "ws://relay.example:3000", + "ws://relay.example:3001" + )); + assert!(!relay_urls_equivalent( + "ws://relay.example", + "wss://relay.example" + )); +} + +// ── relay_http_base_url scheme conversion ──────────────────────────────── + +#[test] +fn loopback_ws_localhost_preserves_authority() { + // Tenant host-binding keys off the HTTP Host/authority. The desktop must + // not rewrite localhost to 127.0.0.1, or local dev HTTP calls target a + // different unmapped community than the WebSocket URL. + assert_eq!( + relay_http_base_url("ws://localhost:3000"), + "http://localhost:3000" + ); +} + +#[test] +fn loopback_trailing_slash_removed_authority_preserved() { + assert_eq!( + relay_http_base_url("ws://localhost:3000/"), + "http://localhost:3000" + ); +} + +#[test] +fn remote_wss_host_unchanged() { + assert_eq!( + relay_http_base_url("wss://relay.example.com"), + "https://relay.example.com" + ); +} + +#[test] +fn loopback_ipv4_literal_unchanged() { + assert_eq!( + relay_http_base_url("ws://127.0.0.1:3000"), + "http://127.0.0.1:3000" + ); +} + +#[test] +fn localhost_substring_host_unchanged() { + assert_eq!( + relay_http_base_url("ws://localhost.evil.com:3000"), + "http://localhost.evil.com:3000" + ); +} + +#[test] +fn loopback_wss_localhost_preserves_authority() { + assert_eq!( + relay_http_base_url("wss://localhost:3000"), + "https://localhost:3000" + ); +} + +// ── classify_intercepted_response ──────────────────────────────────────── + +#[test] +fn intercepted_cloudflare_host_returns_some() { + let result = classify_intercepted_response("sqprod.cloudflareaccess.com", "text/html"); + assert!(result.is_some()); + let msg = result.unwrap(); + assert!( + msg.starts_with("relay unreachable:"), + "should have unreachable prefix" + ); + assert!(msg.contains("Cloudflare"), "should mention Cloudflare"); +} + +#[test] +fn intercepted_cloudflare_apex_host_returns_some() { + // The apex domain itself should also match. + let result = classify_intercepted_response("cloudflareaccess.com", "application/json"); + assert!(result.is_some()); + let msg = result.unwrap(); + assert!(msg.starts_with("relay unreachable:")); + assert!(msg.contains("Cloudflare")); +} + +#[test] +fn intercepted_non_cloudflare_html_returns_some() { + let result = + classify_intercepted_response("proxy.corporate.example", "text/html; charset=utf-8"); + assert!(result.is_some()); + let msg = result.unwrap(); + assert!(msg.starts_with("relay unreachable:")); +} + +#[test] +fn normal_relay_json_returns_none() { + let result = classify_intercepted_response("relay.myapp.example.com", "application/json"); + assert!(result.is_none()); +} + +#[test] +fn content_type_case_insensitive() { + // Uppercase content-type must still be detected. + let result = classify_intercepted_response("proxy.example.com", "TEXT/HTML"); + assert!(result.is_some()); + assert!(result.unwrap().starts_with("relay unreachable:")); +} + +#[test] +fn evil_suffix_does_not_match_cloudflare() { + // A host whose suffix happens to contain the Cloudflare string but is + // not actually a subdomain must NOT match. + let result = + classify_intercepted_response("notcloudflareaccess.com.evil.example", "application/json"); + assert!( + result.is_none(), + "false suffix match should not trigger Cloudflare branch" + ); +} + +// classify_request_error requires a real reqwest::Error (not publicly +// constructable) — tested indirectly through integration; skipped here. + +// ── parse_json_response malformed-body contract ────────────────────────── + +#[test] +fn malformed_response_message_stays_off_unreachable_bucket() { + // A reached-but-malformed 2xx body is not a connectivity failure. If this + // message ever regains the "relay unreachable:" prefix, the frontend + // classifier would misroute it as unreachable — pin that it never does. + assert!( + !MALFORMED_RESPONSE_MESSAGE.starts_with("relay unreachable:"), + "malformed-response message must not match the unreachable prefix" + ); +} + +// ── parse_command_response ─────────────────────────────────────────────── + +#[derive(Debug, Deserialize, PartialEq)] +struct ChannelCreated { + channel_id: String, +} + +#[test] +fn parse_command_response_decodes_typed_payload() { + let msg = r#"response:{"channel_id":"abc123"}"#; + let parsed: ChannelCreated = parse_command_response(msg).expect("should parse"); + assert_eq!( + parsed, + ChannelCreated { + channel_id: "abc123".to_string() + } + ); +} + +#[test] +fn parse_command_response_accepts_raw_json_fallback() { + // Backward-compat: relays that emit raw JSON (no prefix) still work. + let msg = r#"{"channel_id":"abc"}"#; + let parsed: ChannelCreated = parse_command_response(msg).expect("fallback parse"); + assert_eq!( + parsed, + ChannelCreated { + channel_id: "abc".to_string() + } + ); +} + +#[test] +fn parse_command_response_rejects_invalid_prefixed_json() { + let msg = "response:not-json"; + let result: Result = parse_command_response(msg); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("response parse failed")); +} + +#[test] +fn parse_command_response_rejects_garbage() { + let msg = "totally not json or response"; + let result: Result = parse_command_response(msg); + assert!(result.is_err()); +} + +// ── build_profile_event ────────────────────────────────────────────────── + +/// Generate a valid NIP-OA auth tag JSON string signed by a fresh owner key +/// and addressed to `agent_keys`. +/// +/// Uses `nostr_compat` (nostr 0.36) for the owner keys because +/// `buzz_sdk_pkg::nip_oa::compute_auth_tag` expects nostr 0.36 types. +/// The agent pubkey is bridged via hex encoding. +fn make_valid_auth_tag(agent_keys: &nostr::Keys) -> String { + let owner_keys = nostr::Keys::generate(); + let agent_pubkey_hex = agent_keys.public_key().to_hex(); + let agent_compat_pubkey = + nostr::PublicKey::from_hex(&agent_pubkey_hex).expect("valid hex pubkey should parse"); + buzz_sdk_pkg::nip_oa::compute_auth_tag(&owner_keys, &agent_compat_pubkey, "") + .expect("compute_auth_tag should not fail with distinct keys") +} + +#[test] +fn profile_event_with_valid_auth_tag() { + let agent_keys = nostr::Keys::generate(); + let tag_json = make_valid_auth_tag(&agent_keys); + let event = build_profile_event(&agent_keys, "TestBot", None, Some(&tag_json)) + .expect("should succeed with a valid auth tag"); + + // Exactly one "auth" tag must be present. + let auth_tags: Vec<_> = event + .tags + .iter() + .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) + .collect(); + assert_eq!(auth_tags.len(), 1, "expected exactly 1 auth tag"); + + // Must be a kind:0 (Metadata) event. + assert_eq!(event.kind, nostr::Kind::Metadata); +} + +#[test] +fn profile_event_without_auth_tag() { + let agent_keys = nostr::Keys::generate(); + let event = build_profile_event(&agent_keys, "TestBot", None, None) + .expect("should succeed without an auth tag"); + + // No "auth" tags should be present. + let auth_tags: Vec<_> = event + .tags + .iter() + .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) + .collect(); + assert_eq!(auth_tags.len(), 0, "expected no auth tags"); + + assert_eq!(event.kind, nostr::Kind::Metadata); +} + +#[test] +fn profile_event_rejects_invalid_auth_tag() { + let agent_keys = nostr::Keys::generate(); + // Structurally valid JSON array but with a bogus signature — verification must fail. + let bad_json = format!(r#"["auth","{}","","{}"]"#, "a".repeat(64), "b".repeat(128)); + let result = build_profile_event(&agent_keys, "TestBot", None, Some(&bad_json)); + assert!(result.is_err(), "should reject an invalid auth tag"); + assert!( + result.unwrap_err().contains("verification failed"), + "error message should mention verification failure" + ); +} diff --git a/desktop/src/features/agents/agentRelayScope.test.mjs b/desktop/src/features/agents/agentRelayScope.test.mjs index dc2a72802..a8ed43837 100644 --- a/desktop/src/features/agents/agentRelayScope.test.mjs +++ b/desktop/src/features/agents/agentRelayScope.test.mjs @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; import test from "node:test"; import { @@ -15,8 +16,31 @@ const RELAY_B = "wss://relay-b.example.com"; // ── normalizeRelayUrlForCompare ────────────────────────────────────────────── // Must agree with the Rust `normalize_relay_url` (desktop/src-tauri/src/relay.rs) -// because record pins are stamped by the backend and compared here. These -// vectors mirror the Rust unit tests. +// because record pins are stamped by the backend and compared here. The +// agreement contract is the shared fixture below, consumed by both this file +// and the Rust unit tests (`normalize_agrees_with_shared_frontend_fixture`), +// so an edit to either normalizer fails the other side's tests instead of +// shipping a scoping skew. + +test("normalize_agreesWithSharedBackendFixture", () => { + const vectors = JSON.parse( + readFileSync( + new URL( + "../../../fixtures/relay-url-normalization.json", + import.meta.url, + ), + "utf8", + ), + ); + assert.ok(vectors.length > 0, "fixture must not be empty"); + for (const { input, canonical } of vectors) { + assert.equal( + normalizeRelayUrlForCompare(input), + canonical, + `input: ${JSON.stringify(input)}`, + ); + } +}); test("normalize_stripsWhitespaceAndTrailingSlashes", () => { assert.equal( diff --git a/desktop/src/features/agents/agentRelayScope.ts b/desktop/src/features/agents/agentRelayScope.ts index 4736e4370..e0953587e 100644 --- a/desktop/src/features/agents/agentRelayScope.ts +++ b/desktop/src/features/agents/agentRelayScope.ts @@ -13,7 +13,9 @@ * `desktop/src-tauri/src/relay.rs`; the two must agree because record pins * are stamped by the backend and compared here: trim, strip trailing * slashes, lowercase scheme + authority (case-insensitive per RFC 3986), - * preserve any path or query case-sensitively. + * preserve any path or query case-sensitively. The agreement is pinned by + * the shared fixture `desktop/fixtures/relay-url-normalization.json`, + * consumed by both sides' unit tests — extend it with any behavior change. * * Distinct from `normalizeRelayUrl` in `communityStorage.ts` (input * canonicalisation: prepends `wss://`) and in `selfProfileStorage.ts` diff --git a/desktop/src/features/communities/useCommunities.tsx b/desktop/src/features/communities/useCommunities.tsx index c67fce87f..66da498f3 100644 --- a/desktop/src/features/communities/useCommunities.tsx +++ b/desktop/src/features/communities/useCommunities.tsx @@ -235,33 +235,42 @@ function useCommunitiesInternal(): UseCommunitiesReturn { ); if (result.kind === "updated") { + const commit = () => { + setCommunitiesState((prev) => { + const next = prev.map((w) => + w.id === id ? { ...w, ...updates } : w, + ); + saveCommunities(next); + return next; + }); + + if (result.requiresReinit) { + setReinitKey((k) => k + 1); + } + }; + // Agent records are pinned to their home relay, so a relay-URL edit // must re-pin them onto the new URL or they orphan on the old one. - // Fire-and-forget: a failure leaves the pins on the old URL, which - // stays recoverable by re-editing the community. + // The commit — and the reinit-triggered applyCommunity it fires — + // waits for the rebind: apply_workspace on the new URL activates + // start-on-launch agents and marks that relay activated for the + // session, so if it raced ahead of the rebind, agents still pinned + // to the old URL would be silently skipped until app relaunch. A + // rebind failure still commits — the pins stay on the old URL, + // which is recoverable by re-editing the community. const previous = communitiesRef.current.find((w) => w.id === id); if ( previous && updates.relayUrl !== undefined && updates.relayUrl !== previous.relayUrl ) { - rebindAgentRelay(previous.relayUrl, updates.relayUrl).catch( - (error) => { + rebindAgentRelay(previous.relayUrl, updates.relayUrl) + .catch((error) => { console.error("failed to rebind agents to edited relay:", error); - }, - ); - } - - setCommunitiesState((prev) => { - const next = prev.map((w) => - w.id === id ? { ...w, ...updates } : w, - ); - saveCommunities(next); - return next; - }); - - if (result.requiresReinit) { - setReinitKey((k) => k + 1); + }) + .finally(commit); + } else { + commit(); } }