mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): harden the relay-edit and repos-map concurrency edges
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 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
c6c455ba92
commit
1712cbd2ed
@@ -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"
|
||||
}
|
||||
]
|
||||
@@ -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}");
|
||||
|
||||
@@ -273,6 +273,12 @@ fn read_repos_dir_map(nest_root: &Path) -> BTreeMap<String, String> {
|
||||
|
||||
/// 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<String, String>) -> 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<String, String>) -> 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
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<ChannelCreated, _> = 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<ChannelCreated, _> = 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;
|
||||
|
||||
@@ -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<NormalizationVector> = 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<ChannelCreated, _> = 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<ChannelCreated, _> = 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"
|
||||
);
|
||||
}
|
||||
@@ -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(
|
||||
|
||||
@@ -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`
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user