From 91bd81cfa39ad67ed5760b1f1fc6c365624afeaf Mon Sep 17 00:00:00 2001 From: Ahmet Karapinar Date: Sun, 26 Jul 2026 00:45:42 -0400 Subject: [PATCH 1/4] fix(core): fold loopback host spellings in tenant resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `relay::normalize_relay_url` rewrites loopback relay URLs to 127.0.0.1, and clients rely on that canonical form. `tenant::normalize_host` — which keys community lookup from the request Host — did not fold loopback, so the two disagreed and a loopback deployment split into one reachable tenant and one unreachable one. Concretely, with the shipped `.env.example` (`RELAY_URL=ws://localhost:3000`) the relay seeds `communities.host = localhost:3000`. Desktop connects fine using that spelling, but the managed-agent harness is handed the canonical `ws://127.0.0.1:3000` and the relay rejects it at row-zero binding with a generic 404, so agents never connect. No community URL makes both work. Fold `localhost`, 127.0.0.0/8 and ::1 to 127.0.0.1 in `normalize_host`, keeping any non-default port. Every loopback spelling addresses the same machine, so collapsing them cannot widen access across a host boundary; it only stops one deployment splitting into several tenants. A malformed authority must never fold: `split_host_port` now returns None for an unterminated bracket, trailing junk after `]`, or a non-numeric/empty port, so input like `[::1]evil` stays unmatched and fails closed instead of resolving to the loopback community. Migration 0025 rewrites existing loopback `communities.host` rows to the new canonical key. Without it the upgrade strands data: `lower('localhost:3000')` does not conflict with `lower('127.0.0.1:3000')`, so startup would insert a second community and every existing channel, member and event would stay on the old id while requests bound to the new empty one. Rows that would collide fail the migration with both hosts named, rather than silently choosing which tenant survives. Signed-off-by: Ahmet Karapinar --- crates/buzz-core/src/tenant.rs | 180 +++++++++++++++++- .../src/handlers/community_provisioning.rs | 16 +- crates/buzz-relay/src/tenant.rs | 43 ++++- .../0025_fold_loopback_community_hosts.sql | 60 ++++++ 4 files changed, 284 insertions(+), 15 deletions(-) create mode 100644 migrations/0025_fold_loopback_community_hosts.sql diff --git a/crates/buzz-core/src/tenant.rs b/crates/buzz-core/src/tenant.rs index f7894a6999..bd6b67336c 100644 --- a/crates/buzz-core/src/tenant.rs +++ b/crates/buzz-core/src/tenant.rs @@ -112,7 +112,11 @@ impl TenantContext { /// - strip a single trailing dot (the FQDN root label); /// - strip a default port suffix (`:80`, `:443`) — non-default ports are kept, /// since a deployment may legitimately serve different communities on -/// different ports of the same name. +/// different ports of the same name; +/// - fold loopback spellings (`localhost`, `127.0.0.0/8`, `::1`) to +/// `127.0.0.1`, keeping any non-default port. This matches the loopback +/// rewrite in [`crate::relay::normalize_relay_url`], so a host header and a +/// canonicalized relay URL for the same loopback deployment agree. /// /// The input is trimmed of surrounding whitespace. An empty result (e.g. the /// caller passed `""`) is returned as-is; resolution treats an empty or @@ -134,9 +138,74 @@ pub fn normalize_host(host: &str) -> String { if let Some(stripped) = host.strip_suffix('.') { host = stripped.to_string(); } + // Fold loopback spellings onto one key, mirroring the client-side + // canonicalization in `crate::relay::normalize_relay_url`. All loopback + // spellings address the same machine, so collapsing them cannot widen + // access across a host boundary — but leaving them distinct splits one + // loopback deployment into several unreachable tenants. + if let Some((candidate, port)) = split_host_port(&host) { + if is_loopback_host(candidate) { + host = match port { + Some(port) => format!("{LOOPBACK_HOST}:{port}"), + None => LOOPBACK_HOST.to_string(), + }; + } + } host } +/// Canonical spelling every loopback host folds to. Matches the host +/// `crate::relay::normalize_relay_url` rewrites loopback relay URLs to. +const LOOPBACK_HOST: &str = "127.0.0.1"; + +/// Split an authority into its host and optional port, keeping bracketed IPv6 +/// literals intact (`[::1]:3000` becomes `("::1", Some("3000"))`). +/// +/// Returns `None` when the input is not a well-formed authority. The caller +/// must then leave the value untouched: folding a malformed authority would +/// turn input that should fail closed as an unmapped host into a resolvable +/// community key. Rejected here: an unterminated bracket (`[::1`), trailing +/// junk after the bracket (`[::1]evil`), and a non-numeric or empty port +/// (`localhost:abc`). +fn split_host_port(authority: &str) -> Option<(&str, Option<&str>)> { + let (host, port) = if let Some(rest) = authority.strip_prefix('[') { + // Only `[addr]` and `[addr]:port` are well-formed bracketed forms. + let (address, remainder) = rest.split_once(']')?; + if remainder.is_empty() { + (address, None) + } else { + (address, Some(remainder.strip_prefix(':')?)) + } + } else { + match authority.rsplit_once(':') { + // A bare (unbracketed) IPv6 literal has several colons and no port. + Some((head, port)) if !head.contains(':') => (head, Some(port)), + _ => (authority, None), + } + }; + match port { + Some(port) if port.is_empty() || !port.bytes().all(|byte| byte.is_ascii_digit()) => None, + _ => Some((host, port)), + } +} + +/// Whether a host names the loopback interface, by any spelling. +/// +/// Mirrors the loopback test in `crate::relay::normalize_relay_url`: the +/// `localhost` name, any address in `127.0.0.0/8`, and the IPv6 `::1`. +fn is_loopback_host(host: &str) -> bool { + if host.eq_ignore_ascii_case("localhost") { + return true; + } + if let Ok(address) = host.parse::() { + return address.is_loopback(); + } + if let Ok(address) = host.parse::() { + return address.is_loopback(); + } + false +} + /// Extract the authority (host plus an explicit non-default port, if present) /// from a relay URL in the same normalized shape as request `Host` headers and /// `communities.host`. @@ -221,8 +290,90 @@ mod tests { #[test] fn normalize_host_leaves_ipv6_literal_intact() { // IPv6 literals contain colons but no trailing default-port suffix. - assert_eq!(normalize_host("[::1]"), "[::1]"); - assert_eq!(normalize_host("[::1]:443"), "[::1]"); + // A non-loopback literal is used here because loopback spellings fold + // to 127.0.0.1 — see `normalize_host_folds_loopback_spellings`. + assert_eq!(normalize_host("[2001:db8::1]"), "[2001:db8::1]"); + assert_eq!(normalize_host("[2001:db8::1]:443"), "[2001:db8::1]"); + assert_eq!(normalize_host("[2001:db8::1]:8443"), "[2001:db8::1]:8443"); + } + + #[test] + fn normalize_host_folds_loopback_spellings() { + // Every spelling of the loopback interface is the SAME tenant. + for variant in ["localhost", "LocalHost", "127.0.0.1", "127.1.2.3", "[::1]"] { + assert_eq!(normalize_host(variant), "127.0.0.1", "variant: {variant}"); + } + // A non-default port still selects a distinct community, so it is kept. + for variant in ["localhost:3000", "127.0.0.1:3000", "[::1]:3000"] { + assert_eq!( + normalize_host(variant), + "127.0.0.1:3000", + "variant: {variant}" + ); + } + // Default ports are stripped before folding, so these collapse together. + assert_eq!(normalize_host("localhost:80"), "127.0.0.1"); + assert_eq!(normalize_host("[::1]:443"), "127.0.0.1"); + } + + #[test] + fn normalize_host_does_not_fold_malformed_authority() { + // A malformed authority must never fold onto a resolvable key: it has + // to stay unmatched so `bind_community` fails closed. Without this, + // `Host: [::1]evil` would resolve to the loopback community. + // Each of these passes through untouched, so it cannot match a stored + // `communities.host` and resolution rejects it as an unmapped host. + for malformed in [ + "[::1]evil", + "[::1]xyz:3000", + "[::1", + "[::1]:", + "[::1]:port", + "localhost:abc", + "127.0.0.1:", + ] { + assert_eq!( + normalize_host(malformed), + malformed, + "malformed authority must not be rewritten: {malformed}" + ); + } + } + + #[test] + fn normalize_host_does_not_fold_non_loopback() { + // Only the loopback interface folds — nothing else may be collapsed + // onto another tenant's key. + assert_eq!(normalize_host("localhost.example"), "localhost.example"); + assert_eq!(normalize_host("notlocalhost"), "notlocalhost"); + assert_eq!(normalize_host("10.0.0.1"), "10.0.0.1"); + assert_eq!(normalize_host("128.0.0.1"), "128.0.0.1"); + assert_eq!(normalize_host("relay.example:3000"), "relay.example:3000"); + } + + #[test] + fn normalize_host_agrees_with_relay_url_canonicalization() { + // The invariant this whole fold exists to hold: clients canonicalize a + // relay URL with `relay::normalize_relay_url` (which rewrites loopback + // to 127.0.0.1), while tenant resolution keys on the request `Host`. + // If the two disagree, one loopback deployment splits into a reachable + // tenant and an unreachable one. + for url in [ + "ws://localhost:3000", + "ws://127.0.0.1:3000", + "ws://[::1]:3000", + ] { + let canonical = + crate::relay::normalize_relay_url(url).expect("loopback relay URL is valid"); + let authority = canonical + .strip_prefix("ws://") + .expect("normalized ws URL keeps its scheme"); + assert_eq!( + normalize_host(authority), + normalize_host("localhost:3000"), + "url: {url}" + ); + } } #[test] @@ -235,9 +386,11 @@ mod tests { #[test] fn relay_url_authority_keeps_explicit_nondefault_port() { // The default dev seed: startup, bind_deployment_community, and - // buzz-admin must all derive `localhost:3000` (NOT bare `localhost`), - // or the admin lookup misses the community startup seeded. - assert_eq!(relay_url_authority("ws://localhost:3000"), "localhost:3000"); + // buzz-admin must all derive the port (NOT a bare host), or the admin + // lookup misses the community startup seeded. The loopback host folds + // to 127.0.0.1 (see `normalize_host`), but the port is still kept, and + // every derivation path folds identically so they still agree. + assert_eq!(relay_url_authority("ws://localhost:3000"), "127.0.0.1:3000"); assert_eq!( relay_url_authority("wss://relay.example:8443"), "relay.example:8443" @@ -262,8 +415,19 @@ mod tests { #[test] fn relay_url_authority_preserves_ipv6_brackets() { // `host_str()` strips IPv6 brackets and the port; `relay_url_authority` - // must keep both so the authority matches `communities.host`. - assert_eq!(relay_url_authority("ws://[::1]:3000"), "[::1]:3000"); + // must keep both so the authority matches `communities.host`. Checked + // with a non-loopback literal, since loopback folds to 127.0.0.1. + assert_eq!( + relay_url_authority("ws://[2001:db8::1]:3000"), + "[2001:db8::1]:3000" + ); + // The loopback literal folds — and lands on the same authority as the + // other loopback spellings of the same deployment. + assert_eq!(relay_url_authority("ws://[::1]:3000"), "127.0.0.1:3000"); + assert_eq!( + relay_url_authority("ws://[::1]:3000"), + relay_url_authority("ws://localhost:3000") + ); } #[test] diff --git a/crates/buzz-relay/src/handlers/community_provisioning.rs b/crates/buzz-relay/src/handlers/community_provisioning.rs index 3185af8bea..4d0c10829a 100644 --- a/crates/buzz-relay/src/handlers/community_provisioning.rs +++ b/crates/buzz-relay/src/handlers/community_provisioning.rs @@ -361,7 +361,18 @@ mod tests { #[test] fn host_valid_with_port() { - assert!(validate_host("localhost:3000").is_ok()); + assert!(validate_host("127.0.0.1:3000").is_ok()); + assert!(validate_host("relay.example:3000").is_ok()); + } + + #[test] + fn host_rejects_unfolded_loopback_spelling() { + // Loopback folds to 127.0.0.1, so `localhost:3000` is not the + // normalized form. Provisioning stays strict about receiving the + // canonical host, and the error names it — same contract already + // applied to uppercase and trailing-dot spellings. + let error = validate_host("localhost:3000").expect_err("must reject unfolded loopback"); + assert!(error.contains("127.0.0.1:3000"), "error was: {error}"); } #[test] @@ -421,7 +432,8 @@ mod tests { #[test] fn host_accepts_ipv6_bracket_literal() { - assert!(validate_host("[::1]:3000").is_ok()); + // Non-loopback literal keeps its brackets and is accepted as-is. + assert!(validate_host("[2001:db8::1]:3000").is_ok()); } #[test] diff --git a/crates/buzz-relay/src/tenant.rs b/crates/buzz-relay/src/tenant.rs index 88b75f7d6e..5adf9e8e3e 100644 --- a/crates/buzz-relay/src/tenant.rs +++ b/crates/buzz-relay/src/tenant.rs @@ -208,14 +208,17 @@ mod tests { #[tokio::test] async fn deployment_url_keeps_nondefault_port_for_lookup() { - let r = resolver_with("localhost:3000", 42); + // Loopback folds to 127.0.0.1 (see buzz-core::tenant::normalize_host), + // so the community is keyed canonically — but the non-default port is + // still part of the key, which is what this test guards. + let r = resolver_with("127.0.0.1:3000", 42); let ctx = bind_deployment_community(&r, "ws://localhost:3000") .await .expect("deployment host should bind with non-default port"); assert_eq!(ctx.community().as_uuid(), &Uuid::from_u128(42)); - assert_eq!(ctx.host(), "localhost:3000"); + assert_eq!(ctx.host(), "127.0.0.1:3000"); - let wrong = resolver_with("localhost", 42); + let wrong = resolver_with("127.0.0.1", 42); let err = bind_deployment_community(&wrong, "ws://localhost:3000") .await .unwrap_err(); @@ -236,8 +239,38 @@ mod tests { #[test] fn relay_url_authority_preserves_ipv6_brackets() { - assert_eq!(relay_url_authority("ws://[::1]:3000"), "[::1]:3000"); - assert_eq!(relay_url_authority("wss://[::1]:443"), "[::1]"); + // Non-loopback literal: brackets and port are preserved. + assert_eq!( + relay_url_authority("ws://[2001:db8::1]:3000"), + "[2001:db8::1]:3000" + ); + assert_eq!( + relay_url_authority("wss://[2001:db8::1]:443"), + "[2001:db8::1]" + ); + // Loopback literal folds to the canonical loopback host, so every + // spelling of a loopback deployment resolves to one community. + assert_eq!(relay_url_authority("ws://[::1]:3000"), "127.0.0.1:3000"); + assert_eq!(relay_url_authority("wss://[::1]:443"), "127.0.0.1"); + } + + #[tokio::test] + async fn loopback_spellings_bind_the_same_community() { + // Regression: a relay configured with `ws://localhost:PORT` must be + // reachable by every loopback spelling of its own host. Clients + // canonicalize relay URLs to 127.0.0.1, so before the host fold a + // localhost-configured deployment 404'd those callers. + let r = resolver_with("127.0.0.1:3000", 7); + for host in ["localhost:3000", "127.0.0.1:3000", "[::1]:3000"] { + let ctx = bind_community(&r, host) + .await + .unwrap_or_else(|_| panic!("host {host:?} should bind")); + assert_eq!( + ctx.community().as_uuid(), + &Uuid::from_u128(7), + "host {host:?}" + ); + } } #[tokio::test] diff --git a/migrations/0025_fold_loopback_community_hosts.sql b/migrations/0025_fold_loopback_community_hosts.sql new file mode 100644 index 0000000000..09242f1d23 --- /dev/null +++ b/migrations/0025_fold_loopback_community_hosts.sql @@ -0,0 +1,60 @@ +-- Backfill for the loopback fold in `buzz_core::tenant::normalize_host`. +-- +-- `normalize_host` now folds every loopback spelling (`localhost`, +-- 127.0.0.0/8, `::1`) to `127.0.0.1`, so that a host header and a +-- client-canonicalized relay URL for the same loopback deployment agree. +-- `communities.host` stores the already-normalized key, which means rows +-- written under the previous rule are now keyed by a host no request will +-- ever resolve to. +-- +-- Without this backfill the upgrade silently strands data. `communities.host` +-- is unique on `lower(host)`, and `lower('localhost:3000')` does not conflict +-- with `lower('127.0.0.1:3000')`, so `Db::ensure_configured_community` would +-- INSERT a *second* community with a fresh UUID at startup. Every existing +-- channel, member, and event stays attached to the old id while all +-- post-upgrade requests bind to the new, empty one: the deployment comes back +-- up looking wiped, with the original data intact but unreachable. +-- +-- Collisions fail the migration rather than guessing. If a deployment already +-- has two loopback communities that fold onto the same key (say `localhost` +-- and `127.0.0.1`), silently keeping one would strand the other's data. That +-- is a pre-existing misconfiguration on a single-machine host, and the +-- operator has to decide which community survives. + +DO $$ +DECLARE + conflict_report text; +BEGIN + -- Any two rows whose folded hosts are equal would violate + -- idx_communities_host once rewritten. Report every clashing group. + SELECT string_agg(detail, '; ' ORDER BY detail) + INTO conflict_report + FROM ( + SELECT format('[%s] all fold to %L', string_agg(host, ', ' ORDER BY host), fold) + AS detail + FROM ( + SELECT + host, + CASE + WHEN host ~ '^(localhost|127\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}|\[::1\])(:[0-9]+)?$' + THEN '127.0.0.1' || COALESCE(substring(host from ':[0-9]+$'), '') + ELSE host + END AS fold + FROM communities + ) folded + GROUP BY fold + HAVING count(*) > 1 + ) collisions; + + IF conflict_report IS NOT NULL THEN + RAISE EXCEPTION + 'cannot fold loopback community hosts: % . Merge or remove the duplicate communities so each folded host is unique, then re-run the migration.', + conflict_report + USING HINT = 'Loopback spellings (localhost, 127.0.0.0/8, ::1) now share one community key.'; + END IF; + + UPDATE communities + SET host = '127.0.0.1' || COALESCE(substring(host from ':[0-9]+$'), '') + WHERE host ~ '^(localhost|127\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}|\[::1\])(:[0-9]+)?$' + AND host <> '127.0.0.1' || COALESCE(substring(host from ':[0-9]+$'), ''); +END $$; From 2cd48c50b8133b5eb728505b7b982f71d1cc3ddc Mon Sep 17 00:00:00 2001 From: Ahmet Karapinar Date: Sun, 26 Jul 2026 13:05:37 -0400 Subject: [PATCH 2/4] Revert "fix(core): fold loopback host spellings in tenant resolution" This reverts the relay-side loopback fold. Review surfaced that the relay's strictness about loopback spellings is deliberate, not an oversight, so relaxing it is the wrong fix for this bug. `crates/buzz-auth/src/nip98.rs` documents the position explicitly under "No loopback aliasing", and `loopback_aliases_are_distinct_hosts` enforces it: NIP-98 clients sign the exact URL they call, and the relay rebuilds the expected URL from the resolved tenant host. Folding loopback in `normalize_host` therefore made the relay expect `127.0.0.1` while clients such as buzz-cli sign `localhost`, breaking authenticated bridge calls (/query, /events, /count, invites, git) for anyone on the default relay URL. The fold also required a data migration rewriting `communities.host`, which widened the blast radius to every existing deployment for a defect that only affects launching agents. The actual defect is in the desktop app: it hands the agent harness a canonicalized identity key as a dial address, so the harness connects to a host the operator never configured. The next commit fixes it there, which touches no tenancy code, needs no migration, and leaves the documented no-aliasing property intact. Signed-off-by: Ahmet Karapinar --- crates/buzz-core/src/tenant.rs | 180 +----------------- .../src/handlers/community_provisioning.rs | 16 +- crates/buzz-relay/src/tenant.rs | 43 +---- .../0025_fold_loopback_community_hosts.sql | 60 ------ 4 files changed, 15 insertions(+), 284 deletions(-) delete mode 100644 migrations/0025_fold_loopback_community_hosts.sql diff --git a/crates/buzz-core/src/tenant.rs b/crates/buzz-core/src/tenant.rs index bd6b67336c..f7894a6999 100644 --- a/crates/buzz-core/src/tenant.rs +++ b/crates/buzz-core/src/tenant.rs @@ -112,11 +112,7 @@ impl TenantContext { /// - strip a single trailing dot (the FQDN root label); /// - strip a default port suffix (`:80`, `:443`) — non-default ports are kept, /// since a deployment may legitimately serve different communities on -/// different ports of the same name; -/// - fold loopback spellings (`localhost`, `127.0.0.0/8`, `::1`) to -/// `127.0.0.1`, keeping any non-default port. This matches the loopback -/// rewrite in [`crate::relay::normalize_relay_url`], so a host header and a -/// canonicalized relay URL for the same loopback deployment agree. +/// different ports of the same name. /// /// The input is trimmed of surrounding whitespace. An empty result (e.g. the /// caller passed `""`) is returned as-is; resolution treats an empty or @@ -138,74 +134,9 @@ pub fn normalize_host(host: &str) -> String { if let Some(stripped) = host.strip_suffix('.') { host = stripped.to_string(); } - // Fold loopback spellings onto one key, mirroring the client-side - // canonicalization in `crate::relay::normalize_relay_url`. All loopback - // spellings address the same machine, so collapsing them cannot widen - // access across a host boundary — but leaving them distinct splits one - // loopback deployment into several unreachable tenants. - if let Some((candidate, port)) = split_host_port(&host) { - if is_loopback_host(candidate) { - host = match port { - Some(port) => format!("{LOOPBACK_HOST}:{port}"), - None => LOOPBACK_HOST.to_string(), - }; - } - } host } -/// Canonical spelling every loopback host folds to. Matches the host -/// `crate::relay::normalize_relay_url` rewrites loopback relay URLs to. -const LOOPBACK_HOST: &str = "127.0.0.1"; - -/// Split an authority into its host and optional port, keeping bracketed IPv6 -/// literals intact (`[::1]:3000` becomes `("::1", Some("3000"))`). -/// -/// Returns `None` when the input is not a well-formed authority. The caller -/// must then leave the value untouched: folding a malformed authority would -/// turn input that should fail closed as an unmapped host into a resolvable -/// community key. Rejected here: an unterminated bracket (`[::1`), trailing -/// junk after the bracket (`[::1]evil`), and a non-numeric or empty port -/// (`localhost:abc`). -fn split_host_port(authority: &str) -> Option<(&str, Option<&str>)> { - let (host, port) = if let Some(rest) = authority.strip_prefix('[') { - // Only `[addr]` and `[addr]:port` are well-formed bracketed forms. - let (address, remainder) = rest.split_once(']')?; - if remainder.is_empty() { - (address, None) - } else { - (address, Some(remainder.strip_prefix(':')?)) - } - } else { - match authority.rsplit_once(':') { - // A bare (unbracketed) IPv6 literal has several colons and no port. - Some((head, port)) if !head.contains(':') => (head, Some(port)), - _ => (authority, None), - } - }; - match port { - Some(port) if port.is_empty() || !port.bytes().all(|byte| byte.is_ascii_digit()) => None, - _ => Some((host, port)), - } -} - -/// Whether a host names the loopback interface, by any spelling. -/// -/// Mirrors the loopback test in `crate::relay::normalize_relay_url`: the -/// `localhost` name, any address in `127.0.0.0/8`, and the IPv6 `::1`. -fn is_loopback_host(host: &str) -> bool { - if host.eq_ignore_ascii_case("localhost") { - return true; - } - if let Ok(address) = host.parse::() { - return address.is_loopback(); - } - if let Ok(address) = host.parse::() { - return address.is_loopback(); - } - false -} - /// Extract the authority (host plus an explicit non-default port, if present) /// from a relay URL in the same normalized shape as request `Host` headers and /// `communities.host`. @@ -290,90 +221,8 @@ mod tests { #[test] fn normalize_host_leaves_ipv6_literal_intact() { // IPv6 literals contain colons but no trailing default-port suffix. - // A non-loopback literal is used here because loopback spellings fold - // to 127.0.0.1 — see `normalize_host_folds_loopback_spellings`. - assert_eq!(normalize_host("[2001:db8::1]"), "[2001:db8::1]"); - assert_eq!(normalize_host("[2001:db8::1]:443"), "[2001:db8::1]"); - assert_eq!(normalize_host("[2001:db8::1]:8443"), "[2001:db8::1]:8443"); - } - - #[test] - fn normalize_host_folds_loopback_spellings() { - // Every spelling of the loopback interface is the SAME tenant. - for variant in ["localhost", "LocalHost", "127.0.0.1", "127.1.2.3", "[::1]"] { - assert_eq!(normalize_host(variant), "127.0.0.1", "variant: {variant}"); - } - // A non-default port still selects a distinct community, so it is kept. - for variant in ["localhost:3000", "127.0.0.1:3000", "[::1]:3000"] { - assert_eq!( - normalize_host(variant), - "127.0.0.1:3000", - "variant: {variant}" - ); - } - // Default ports are stripped before folding, so these collapse together. - assert_eq!(normalize_host("localhost:80"), "127.0.0.1"); - assert_eq!(normalize_host("[::1]:443"), "127.0.0.1"); - } - - #[test] - fn normalize_host_does_not_fold_malformed_authority() { - // A malformed authority must never fold onto a resolvable key: it has - // to stay unmatched so `bind_community` fails closed. Without this, - // `Host: [::1]evil` would resolve to the loopback community. - // Each of these passes through untouched, so it cannot match a stored - // `communities.host` and resolution rejects it as an unmapped host. - for malformed in [ - "[::1]evil", - "[::1]xyz:3000", - "[::1", - "[::1]:", - "[::1]:port", - "localhost:abc", - "127.0.0.1:", - ] { - assert_eq!( - normalize_host(malformed), - malformed, - "malformed authority must not be rewritten: {malformed}" - ); - } - } - - #[test] - fn normalize_host_does_not_fold_non_loopback() { - // Only the loopback interface folds — nothing else may be collapsed - // onto another tenant's key. - assert_eq!(normalize_host("localhost.example"), "localhost.example"); - assert_eq!(normalize_host("notlocalhost"), "notlocalhost"); - assert_eq!(normalize_host("10.0.0.1"), "10.0.0.1"); - assert_eq!(normalize_host("128.0.0.1"), "128.0.0.1"); - assert_eq!(normalize_host("relay.example:3000"), "relay.example:3000"); - } - - #[test] - fn normalize_host_agrees_with_relay_url_canonicalization() { - // The invariant this whole fold exists to hold: clients canonicalize a - // relay URL with `relay::normalize_relay_url` (which rewrites loopback - // to 127.0.0.1), while tenant resolution keys on the request `Host`. - // If the two disagree, one loopback deployment splits into a reachable - // tenant and an unreachable one. - for url in [ - "ws://localhost:3000", - "ws://127.0.0.1:3000", - "ws://[::1]:3000", - ] { - let canonical = - crate::relay::normalize_relay_url(url).expect("loopback relay URL is valid"); - let authority = canonical - .strip_prefix("ws://") - .expect("normalized ws URL keeps its scheme"); - assert_eq!( - normalize_host(authority), - normalize_host("localhost:3000"), - "url: {url}" - ); - } + assert_eq!(normalize_host("[::1]"), "[::1]"); + assert_eq!(normalize_host("[::1]:443"), "[::1]"); } #[test] @@ -386,11 +235,9 @@ mod tests { #[test] fn relay_url_authority_keeps_explicit_nondefault_port() { // The default dev seed: startup, bind_deployment_community, and - // buzz-admin must all derive the port (NOT a bare host), or the admin - // lookup misses the community startup seeded. The loopback host folds - // to 127.0.0.1 (see `normalize_host`), but the port is still kept, and - // every derivation path folds identically so they still agree. - assert_eq!(relay_url_authority("ws://localhost:3000"), "127.0.0.1:3000"); + // buzz-admin must all derive `localhost:3000` (NOT bare `localhost`), + // or the admin lookup misses the community startup seeded. + assert_eq!(relay_url_authority("ws://localhost:3000"), "localhost:3000"); assert_eq!( relay_url_authority("wss://relay.example:8443"), "relay.example:8443" @@ -415,19 +262,8 @@ mod tests { #[test] fn relay_url_authority_preserves_ipv6_brackets() { // `host_str()` strips IPv6 brackets and the port; `relay_url_authority` - // must keep both so the authority matches `communities.host`. Checked - // with a non-loopback literal, since loopback folds to 127.0.0.1. - assert_eq!( - relay_url_authority("ws://[2001:db8::1]:3000"), - "[2001:db8::1]:3000" - ); - // The loopback literal folds — and lands on the same authority as the - // other loopback spellings of the same deployment. - assert_eq!(relay_url_authority("ws://[::1]:3000"), "127.0.0.1:3000"); - assert_eq!( - relay_url_authority("ws://[::1]:3000"), - relay_url_authority("ws://localhost:3000") - ); + // must keep both so the authority matches `communities.host`. + assert_eq!(relay_url_authority("ws://[::1]:3000"), "[::1]:3000"); } #[test] diff --git a/crates/buzz-relay/src/handlers/community_provisioning.rs b/crates/buzz-relay/src/handlers/community_provisioning.rs index 4d0c10829a..3185af8bea 100644 --- a/crates/buzz-relay/src/handlers/community_provisioning.rs +++ b/crates/buzz-relay/src/handlers/community_provisioning.rs @@ -361,18 +361,7 @@ mod tests { #[test] fn host_valid_with_port() { - assert!(validate_host("127.0.0.1:3000").is_ok()); - assert!(validate_host("relay.example:3000").is_ok()); - } - - #[test] - fn host_rejects_unfolded_loopback_spelling() { - // Loopback folds to 127.0.0.1, so `localhost:3000` is not the - // normalized form. Provisioning stays strict about receiving the - // canonical host, and the error names it — same contract already - // applied to uppercase and trailing-dot spellings. - let error = validate_host("localhost:3000").expect_err("must reject unfolded loopback"); - assert!(error.contains("127.0.0.1:3000"), "error was: {error}"); + assert!(validate_host("localhost:3000").is_ok()); } #[test] @@ -432,8 +421,7 @@ mod tests { #[test] fn host_accepts_ipv6_bracket_literal() { - // Non-loopback literal keeps its brackets and is accepted as-is. - assert!(validate_host("[2001:db8::1]:3000").is_ok()); + assert!(validate_host("[::1]:3000").is_ok()); } #[test] diff --git a/crates/buzz-relay/src/tenant.rs b/crates/buzz-relay/src/tenant.rs index 5adf9e8e3e..88b75f7d6e 100644 --- a/crates/buzz-relay/src/tenant.rs +++ b/crates/buzz-relay/src/tenant.rs @@ -208,17 +208,14 @@ mod tests { #[tokio::test] async fn deployment_url_keeps_nondefault_port_for_lookup() { - // Loopback folds to 127.0.0.1 (see buzz-core::tenant::normalize_host), - // so the community is keyed canonically — but the non-default port is - // still part of the key, which is what this test guards. - let r = resolver_with("127.0.0.1:3000", 42); + let r = resolver_with("localhost:3000", 42); let ctx = bind_deployment_community(&r, "ws://localhost:3000") .await .expect("deployment host should bind with non-default port"); assert_eq!(ctx.community().as_uuid(), &Uuid::from_u128(42)); - assert_eq!(ctx.host(), "127.0.0.1:3000"); + assert_eq!(ctx.host(), "localhost:3000"); - let wrong = resolver_with("127.0.0.1", 42); + let wrong = resolver_with("localhost", 42); let err = bind_deployment_community(&wrong, "ws://localhost:3000") .await .unwrap_err(); @@ -239,38 +236,8 @@ mod tests { #[test] fn relay_url_authority_preserves_ipv6_brackets() { - // Non-loopback literal: brackets and port are preserved. - assert_eq!( - relay_url_authority("ws://[2001:db8::1]:3000"), - "[2001:db8::1]:3000" - ); - assert_eq!( - relay_url_authority("wss://[2001:db8::1]:443"), - "[2001:db8::1]" - ); - // Loopback literal folds to the canonical loopback host, so every - // spelling of a loopback deployment resolves to one community. - assert_eq!(relay_url_authority("ws://[::1]:3000"), "127.0.0.1:3000"); - assert_eq!(relay_url_authority("wss://[::1]:443"), "127.0.0.1"); - } - - #[tokio::test] - async fn loopback_spellings_bind_the_same_community() { - // Regression: a relay configured with `ws://localhost:PORT` must be - // reachable by every loopback spelling of its own host. Clients - // canonicalize relay URLs to 127.0.0.1, so before the host fold a - // localhost-configured deployment 404'd those callers. - let r = resolver_with("127.0.0.1:3000", 7); - for host in ["localhost:3000", "127.0.0.1:3000", "[::1]:3000"] { - let ctx = bind_community(&r, host) - .await - .unwrap_or_else(|_| panic!("host {host:?} should bind")); - assert_eq!( - ctx.community().as_uuid(), - &Uuid::from_u128(7), - "host {host:?}" - ); - } + assert_eq!(relay_url_authority("ws://[::1]:3000"), "[::1]:3000"); + assert_eq!(relay_url_authority("wss://[::1]:443"), "[::1]"); } #[tokio::test] diff --git a/migrations/0025_fold_loopback_community_hosts.sql b/migrations/0025_fold_loopback_community_hosts.sql deleted file mode 100644 index 09242f1d23..0000000000 --- a/migrations/0025_fold_loopback_community_hosts.sql +++ /dev/null @@ -1,60 +0,0 @@ --- Backfill for the loopback fold in `buzz_core::tenant::normalize_host`. --- --- `normalize_host` now folds every loopback spelling (`localhost`, --- 127.0.0.0/8, `::1`) to `127.0.0.1`, so that a host header and a --- client-canonicalized relay URL for the same loopback deployment agree. --- `communities.host` stores the already-normalized key, which means rows --- written under the previous rule are now keyed by a host no request will --- ever resolve to. --- --- Without this backfill the upgrade silently strands data. `communities.host` --- is unique on `lower(host)`, and `lower('localhost:3000')` does not conflict --- with `lower('127.0.0.1:3000')`, so `Db::ensure_configured_community` would --- INSERT a *second* community with a fresh UUID at startup. Every existing --- channel, member, and event stays attached to the old id while all --- post-upgrade requests bind to the new, empty one: the deployment comes back --- up looking wiped, with the original data intact but unreachable. --- --- Collisions fail the migration rather than guessing. If a deployment already --- has two loopback communities that fold onto the same key (say `localhost` --- and `127.0.0.1`), silently keeping one would strand the other's data. That --- is a pre-existing misconfiguration on a single-machine host, and the --- operator has to decide which community survives. - -DO $$ -DECLARE - conflict_report text; -BEGIN - -- Any two rows whose folded hosts are equal would violate - -- idx_communities_host once rewritten. Report every clashing group. - SELECT string_agg(detail, '; ' ORDER BY detail) - INTO conflict_report - FROM ( - SELECT format('[%s] all fold to %L', string_agg(host, ', ' ORDER BY host), fold) - AS detail - FROM ( - SELECT - host, - CASE - WHEN host ~ '^(localhost|127\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}|\[::1\])(:[0-9]+)?$' - THEN '127.0.0.1' || COALESCE(substring(host from ':[0-9]+$'), '') - ELSE host - END AS fold - FROM communities - ) folded - GROUP BY fold - HAVING count(*) > 1 - ) collisions; - - IF conflict_report IS NOT NULL THEN - RAISE EXCEPTION - 'cannot fold loopback community hosts: % . Merge or remove the duplicate communities so each folded host is unique, then re-run the migration.', - conflict_report - USING HINT = 'Loopback spellings (localhost, 127.0.0.0/8, ::1) now share one community key.'; - END IF; - - UPDATE communities - SET host = '127.0.0.1' || COALESCE(substring(host from ':[0-9]+$'), '') - WHERE host ~ '^(localhost|127\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}|\[::1\])(:[0-9]+)?$' - AND host <> '127.0.0.1' || COALESCE(substring(host from ':[0-9]+$'), ''); -END $$; From 21d4a02f968def697075bc3359ddd26dda294e7a Mon Sep 17 00:00:00 2001 From: Ahmet Karapinar Date: Sun, 26 Jul 2026 13:18:52 -0400 Subject: [PATCH 3/4] fix(desktop): dial the requested relay URL when spawning an agent harness `ManagedAgentRuntimeKey` is an identity: `ManagedAgentRuntimeKey::new` canonicalizes the relay URL (folding loopback to 127.0.0.1) so `runtime_id()` hashes to a stable on-disk path for one pair regardless of how the operator spelled the host. That is correct for keying. `spawn_agent_child` then reused that canonical key as the address the child connects to, and all three callers passed `&key.relay_url`. The relay resolves a community from the literal request Host and fails closed on an unmapped one, so with the shipped default (`RELAY_URL=ws://localhost:3000`, seeding `communities.host = localhost:3000`) the harness dialed `ws://127.0.0.1:3000` and was rejected with a generic 404: WARN buzz_acp::relay: initial relay connect failed with terminal error: WebSocket error: HTTP error: 404 Not Found Desktop's own socket connects, because it dials the URL the operator entered, so agents fail while the app looks healthy and no error surfaces in the UI. There is no community URL that satisfies both: `localhost` breaks agents, `127.0.0.1` breaks the desktop socket. Dial the URL as requested. The key stays canonical, so `runtime_id()` and every existing runtime directory are unchanged, and the pin the call-site comment describes is preserved: the child still connects to exactly one relay, now the one the operator configured. Deliberately not fixed by relaxing the relay: `crates/buzz-auth/src/nip98.rs` documents "No loopback aliasing" and `loopback_aliases_are_distinct_hosts` enforces it, because NIP-98 clients sign the exact URL they call. Folding loopback server-side breaks authenticated bridge calls and needs a migration over `communities.host`; fixing the caller needs neither. Signed-off-by: Ahmet Karapinar --- .../src-tauri/src/managed_agents/restore.rs | 4 +- .../src-tauri/src/managed_agents/runtime.rs | 17 +++++++-- .../src/managed_agents/runtime_commands.rs | 3 +- .../src/managed_agents/runtime_types.rs | 37 +++++++++++++++++++ 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index fab481e12b..dfd456cbb3 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -320,10 +320,12 @@ pub async fn restore_managed_agents_on_launch( // mid-turn session is not resumed by an // eager child — and silently reintroduces // N idle brains on every launch. + // Requested URL, not the canonical + // key: the child dials this. spawn_agent_child( app, record, - &key.relay_url, + &relay_url, true, owner_hex_ref, ) diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 44cee49aeb..40ba9824a8 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1692,9 +1692,17 @@ pub fn spawn_agent_child( .map(|p| p.display().to_string()) .unwrap_or_else(|| effective_command.clone()); - // The caller supplies the explicit canonical pair relay. This is the only - // relay this child may connect to, regardless of the record/workspace default. - let effective_relay_url = runtime_key.relay_url.clone(); + // The caller supplies the explicit pair relay. This is the only relay this + // child may connect to, regardless of the record/workspace default. + // + // Dial the URL as requested, NOT `runtime_key.relay_url`. The key is an + // identity: `ManagedAgentRuntimeKey::new` canonicalizes it (folding loopback + // to 127.0.0.1) so `runtime_id()` stays stable, which is right for keying + // and wrong for addressing. The relay resolves a community from the literal + // request Host and fails closed on an unmapped one, so a deployment + // configured as `ws://localhost:PORT` rejects a canonicalized + // `ws://127.0.0.1:PORT` with a generic 404 and the harness never connects. + let effective_relay_url = relay_url.to_string(); // Augment PATH for DMG launches so child processes can find: // - bundled CLI via ~/.local/bin symlink @@ -2134,7 +2142,8 @@ pub fn start_managed_agent_process( // Scalar PIDs are migration-only and never establish pair liveness. record.runtime_pid = None; - let mut process = spawn_agent_child(app, record, &key.relay_url, false, owner_hex)?; + // Pass the requested URL, not the canonical key: the child dials this. + let mut process = spawn_agent_child(app, record, &relay_url, false, owner_hex)?; let now = now_iso(); let receipt = super::ManagedAgentRuntimeReceipt { key: key.clone(), diff --git a/desktop/src-tauri/src/managed_agents/runtime_commands.rs b/desktop/src-tauri/src/managed_agents/runtime_commands.rs index c0e55184b1..11e0a9e01b 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_commands.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_commands.rs @@ -283,7 +283,8 @@ fn start_pair( .lock() .ok() .map(|keys| keys.public_key().to_hex()); - let mut process = spawn_agent_child(&app, record, &key.relay_url, lazy, owner.as_deref())?; + // Pass the requested URL, not the canonical key: the child dials this. + let mut process = spawn_agent_child(&app, record, &relay_url, lazy, owner.as_deref())?; let now = crate::util::now_iso(); let receipt = ManagedAgentRuntimeReceipt { key: key.clone(), diff --git a/desktop/src-tauri/src/managed_agents/runtime_types.rs b/desktop/src-tauri/src/managed_agents/runtime_types.rs index 4862cedbae..2b05ac8f2e 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_types.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_types.rs @@ -31,6 +31,43 @@ impl ManagedAgentRuntimeKey { } } +#[cfg(test)] +mod tests { + use super::ManagedAgentRuntimeKey; + + #[test] + fn runtime_key_relay_url_is_canonical_identity_not_a_dial_address() { + // `relay_url` here is canonicalized (loopback folds to 127.0.0.1) so + // `runtime_id()` is stable for one pair no matter how the operator spelled + // the host. That is correct for keying, and it is exactly why this field + // must never be handed to the harness as the URL to connect to: the relay + // resolves a community from the literal request Host and fails closed on + // an unmapped one, so a deployment configured as `ws://localhost:PORT` + // rejects `ws://127.0.0.1:PORT` with a generic 404. `spawn_agent_child` + // dials the URL as requested; see the comment there. + let pubkey = "a".repeat(64); + for spelling in [ + "ws://localhost:3000", + "ws://127.0.0.1:3000", + "ws://[::1]:3000", + ] { + let key = ManagedAgentRuntimeKey::new(pubkey.clone(), spelling) + .expect("loopback relay URL should be accepted"); + assert_eq!( + key.relay_url, "ws://127.0.0.1:3000", + "spelling {spelling} should canonicalize" + ); + } + + // One identity across spellings, so the on-disk runtime path is stable. + let named = ManagedAgentRuntimeKey::new(pubkey.clone(), "ws://localhost:3000") + .expect("valid relay URL"); + let numeric = ManagedAgentRuntimeKey::new(pubkey.clone(), "ws://127.0.0.1:3000") + .expect("valid relay URL"); + assert_eq!(named.runtime_id(), numeric.runtime_id()); + } +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum ManagedAgentRuntimeLifecycle { From a3e4923a911e3e245b0353e24a20234b094ee50a Mon Sep 17 00:00:00 2001 From: Ahmet Karapinar Date: Sun, 26 Jul 2026 15:02:54 -0400 Subject: [PATCH 4/4] fix(desktop): use the requested relay URL for the access probe and reconciliation Follow-up to the previous commit, which fixed `spawn_agent_child` and its three direct callers but left the layer above them canonicalized. `probe_agent_relay_access` built its HTTP base from `key.relay_url`, and the successful-probe branch called `start_pair` with `key.relay_url` even though the requested URL was already in scope and used two lines later for the status row. Reconciliation and post-create bootstrap therefore still probed, and on success dialed, the canonical host, reproducing the 404 the previous commit set out to prevent. Both now use the requested URL. Also restores the spawn config fingerprint to the canonical pair URL. The previous commit redefined `effective_relay_url` to the requested spelling, and that binding was serving two unrelated purposes: the address to dial and the input to `spawn_config_hash`. `needs_restart` recomputes that hash from `key.relay_url`, so the two sides disagreed for any host the key folds and reported needs_restart permanently. The dial and the fingerprint are separate concerns: dial the request, fingerprint the identity. This matches upstream behaviour for the fingerprint, changing only the dial. Signed-off-by: Ahmet Karapinar --- desktop/src-tauri/src/managed_agents/runtime.rs | 12 +++++++++--- .../src-tauri/src/managed_agents/runtime_commands.rs | 9 +++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 40ba9824a8..72e4c3f422 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -2053,13 +2053,19 @@ pub fn spawn_agent_child( // Stamp the effective spawn config so the summary builder can flag // needs_restart when disk state drifts from what this process runs. - // `effective_relay_url` is already resolved, and resolution is idempotent, - // so it serves as the workspace-relay input here. + // + // Hash the CANONICAL pair URL, not the dialed one. `needs_restart` + // recomputes this hash from `key.relay_url` (the canonical identity), so + // feeding the requested spelling here would make the two disagree for any + // host the key folds (e.g. `localhost` vs `127.0.0.1`) and report + // needs_restart forever. The relay a child dials and the config it was + // spawned with are separate concerns: dial the request, fingerprint the + // identity. let spawn_config_hash = super::spawn_hash::spawn_config_hash( record, &personas, &teams, - &effective_relay_url, + &runtime_key.relay_url, &global, ); diff --git a/desktop/src-tauri/src/managed_agents/runtime_commands.rs b/desktop/src-tauri/src/managed_agents/runtime_commands.rs index 11e0a9e01b..e18699969a 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_commands.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_commands.rs @@ -404,7 +404,11 @@ async fn probe_agent_relay_access( let key = ManagedAgentRuntimeKey::new(record.pubkey.clone(), &requested_relay_url)?; let keys = nostr::Keys::parse(record.private_key_nsec.trim()) .map_err(|error| format!("invalid managed-agent key: {error}"))?; - let api_base = crate::relay::relay_http_base_url(&key.relay_url); + // Probe the relay as requested, not via the canonical key: the relay + // resolves a community from the literal Host and fails closed on an + // unmapped one, so probing a canonicalized loopback host 404s on a + // deployment configured with `localhost`. + let api_base = crate::relay::relay_http_base_url(&requested_relay_url); tokio::time::timeout( std::time::Duration::from_secs(10), crate::relay::query_relay_at_with_keys( @@ -505,7 +509,8 @@ pub async fn reconcile_managed_agent_runtimes( Ok((record, key, requested)) => { match start_pair( record.pubkey.clone(), - key.relay_url.clone(), + // Requested URL, not the canonical key: this is dialed. + requested.clone(), true, Some(&record.updated_at), app.clone(),