Skip to content

Commit 659cabd

Browse files
JSKittyclaude
andcommitted
fix(concord): harden fetch_plane per pre-publish review; bump vector-core 0.4.0 / vector_sdk 0.5.0
Pre-publish review of the plane-key auth-fetch path surfaced fixable issues; the core security properties (owner always protected, no cross-account client handoff, escalation fix correct for the in-roster case) held. - Tor: fetch_plane added relays BARE (ConnectionMode::Direct), so under active Tor the plane fetch + its NIP-42 auth as the plane key could connect direct and tie the user's IP to community membership. Now adds via pool().add_relay with the Tor-aware community relay options, matching warm_client. - Cross-swap: clear_plane_pool drained the map but left the generation stamp, so a fetch_plane in flight during an account swap could re-pool a client still authed as the swapped-out account's plane key. Now stamps the live generation on clear. - Confident-empty: fetch_plane returned Ok(empty) when every relay refused/timed out (indistinguishable from a genuinely empty plane). Now Err on zero EOSE. - Pool insert no longer returns the in-use client for disconnect (only real LRU victims), so a raced same-key miss can't tear down the connection it fetches on. - disconnect_clients is runtime-guarded (clear_plane_pool is pub; a bot author may call it off a non-tokio thread). Documented residual (bounded, non-blocking): base_admissible reads the persisted roster, so a superior whose grant hasn't folded yet is unprotected within the grant-propagation window; the owner is always hard-protected and can counter-refound. Version bump is minor: fetch_plane is a new required method on the public Transport trait (breaking for external Transport impls). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d3d7cf1 commit 659cabd

6 files changed

Lines changed: 60 additions & 22 deletions

File tree

crates/Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/vector-core/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "vector-core"
3-
version = "0.3.0"
3+
version = "0.4.0"
44
edition = "2021"
55
description = "Core library for Vector — the single source of truth for all Vector clients, SDKs, and interfaces."
66
license = "MIT"

crates/vector-core/src/community/transport.rs

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -420,11 +420,22 @@ fn plane_pool_key(plane_pk: &str, relays: &[String]) -> String {
420420
}
421421

422422
/// Disconnect the given clients off the hot path (never awaited under the lock).
423+
/// Runtime-guarded: `clear_plane_pool` is public and a bot author may call it from
424+
/// a non-tokio thread — `disconnect` off a live handle if there is one, else drop
425+
/// the client (its background task ends on drop).
423426
fn disconnect_clients(clients: Vec<Client>) {
424-
for c in clients {
425-
tokio::spawn(async move {
426-
let _ = c.disconnect();
427-
});
427+
if clients.is_empty() {
428+
return;
429+
}
430+
match tokio::runtime::Handle::try_current() {
431+
Ok(handle) => {
432+
for c in clients {
433+
handle.spawn(async move {
434+
let _ = c.disconnect();
435+
});
436+
}
437+
}
438+
Err(_) => drop(clients),
428439
}
429440
}
430441

@@ -433,6 +444,11 @@ fn disconnect_clients(clients: Vec<Client>) {
433444
pub fn clear_plane_pool() {
434445
let drained: Vec<Client> = {
435446
let mut g = PLANE_POOL.lock().unwrap_or_else(|e| e.into_inner());
447+
// Stamp the LIVE generation so an insert still in flight from the prior
448+
// generation (a fetch_plane that captured the old value before the swap)
449+
// sees the mismatch and disconnects its client instead of re-pooling one
450+
// still authed as the swapped-out account's plane key.
451+
g.0 = crate::state::current_session_generation();
436452
g.1.drain().map(|(_, p)| p.client).collect()
437453
};
438454
disconnect_clients(drained);
@@ -467,18 +483,19 @@ fn plane_pool_take(generation: u64, key: &str) -> (Option<Client>, Vec<Client>)
467483
}
468484

469485
/// Insert a freshly-built client for `key`, LRU-evicting if over the cap. Returns
470-
/// clients to disconnect (a raced sibling insert, or the LRU victim).
486+
/// ONLY the displaced LRU victim(s) to disconnect — NEVER the just-built `client`.
487+
/// If we don't pool it (swapped mid-build, or a concurrent miss already pooled
488+
/// this key), we return nothing: the caller still uses the client for this one
489+
/// fetch and it closes on drop, so we must not disconnect the connection it's
490+
/// about to run on.
471491
fn plane_pool_insert(generation: u64, key: String, client: Client) -> Vec<Client> {
472492
let mut g = PLANE_POOL.lock().unwrap_or_else(|e| e.into_inner());
473-
if g.0 != generation {
474-
// Swapped mid-build — don't pool into the new generation; caller still uses it once.
475-
return vec![client];
493+
// Swapped mid-build, or a concurrent miss already pooled this key — don't pool
494+
// ours (the caller uses it once, then it drops).
495+
if g.0 != generation || g.1.contains_key(&key) {
496+
return Vec::new();
476497
}
477498
let mut evicted: Vec<Client> = Vec::new();
478-
// A concurrent miss for the same key already inserted — keep theirs, drop ours.
479-
if g.1.contains_key(&key) {
480-
return vec![client];
481-
}
482499
if g.1.len() >= PLANE_POOL_MAX {
483500
if let Some(lru_key) = g.1.iter().min_by_key(|(_, p)| p.last_used).map(|(k, _)| k.clone()) {
484501
if let Some(p) = g.1.remove(&lru_key) {
@@ -1054,8 +1071,12 @@ impl Transport for LiveTransport {
10541071
// connection holds ONE identity; the shared client's is the user's).
10551072
let opts = crate::nostr_client_options().automatic_authentication(true);
10561073
let client = nostr_sdk::Client::builder().signer(plane.clone()).opts(opts).build();
1074+
// Community relay options (GOSSIP|PING + Tor-aware ConnectionMode): a
1075+
// bare add_relay leaves ConnectionMode::Direct, so under active Tor the
1076+
// plane fetch — and the NIP-42 auth AS the plane key — would connect
1077+
// direct and tie the user's IP to community membership.
10571078
for r in &targets {
1058-
let _ = client.add_relay(r.clone()).await;
1079+
let _ = client.pool().add_relay(r.clone(), crate::community_relay_options()).await;
10591080
}
10601081
client.connect().await;
10611082
// Warmup with the gated filter shape triggers each relay's NIP-42
@@ -1072,19 +1093,28 @@ impl Transport for LiveTransport {
10721093

10731094
let mut result: Vec<Event> = Vec::new();
10741095
let mut seen: std::collections::HashSet<EventId> = std::collections::HashSet::new();
1096+
let mut successes = 0usize;
10751097
for r in &targets {
10761098
let res = fetch_relay_eose(&client, r, filter.clone(), self.timeout).await;
10771099
// Feed the shared breaker so this auth path both benefits from AND
10781100
// contributes to the pool-wide dead-relay knowledge.
10791101
breaker_record(r, res.is_ok(), true);
10801102
if let Ok(events) = res {
1103+
successes += 1;
10811104
for e in events {
10821105
if seen.insert(e.id) {
10831106
result.push(e);
10841107
}
10851108
}
10861109
}
10871110
}
1111+
// Zero EOSE = every relay refused/timed out — an honest transient failure,
1112+
// NOT a "the plane is empty" verdict (a genuine empty plane EOSEs with no
1113+
// events, which counts as a success). A confident-empty here could mask a
1114+
// rotation from a caller that concludes absence.
1115+
if successes == 0 {
1116+
return Err(format!("no relay answered the plane fetch (0/{} attempted)", targets.len()));
1117+
}
10881118
// The client stays POOLED (not disconnected) for the next page/epoch/community.
10891119
Ok(result)
10901120
}

crates/vector-core/src/community/v2/service.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3340,9 +3340,17 @@ pub async fn follow_rekeys<T: Transport + ?Sized>(
33403340
let mut cur = community.clone();
33413341
let mut changed = false;
33423342

3343-
// The channel-rotator gates. Loaded once per follow: a roster change lands via
3344-
// follow_control (which the worker runs right after this), so at worst an
3345-
// admin's rotation adopts one pass late — never early.
3343+
// The rotator/admissibility gates read the PERSISTED roster (folded by a prior
3344+
// follow_control; the worker folds control right after this rekey pass). This
3345+
// is "one pass late" for the rotator-AUTHORIZATION direction (a newly-granted
3346+
// admin's rotation adopts a pass late, never early — safe). It is fail-OPEN for
3347+
// the base-admissibility protected-set: a superior whose grant this receiver
3348+
// has not yet folded is not in `roster.grants`, so a non-owner Refounding
3349+
// excluding them can be adopted within that propagation window. Bounded — the
3350+
// owner is ALWAYS hard-protected below (independent of the roster) and can
3351+
// counter-refound; and it is inherent to eventual consistency (one cannot gate
3352+
// on a grant never seen). Tightening this (fold control before the first rekey,
3353+
// or gate non-owner adoption on roster freshness) is a follow-on.
33463354
let roster = crate::db::community::get_community_roles(&cid_hex).unwrap_or_default();
33473355
let banned = crate::db::community::get_community_banlist(&cid_hex).unwrap_or_default();
33483356
let me_hex = me.public_key().to_hex();

crates/vector-sdk/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "vector_sdk"
3-
version = "0.4.1"
3+
version = "0.5.0"
44
edition = "2021"
55
description = "Ergonomic Rust SDK for building Vector bots and clients on top of vector-core."
66
license = "MIT"
@@ -17,7 +17,7 @@ readme = "README.md"
1717
# (vector-core's `tor` feature is off by default) so the SDK's dep tree is light.
1818
# `version` is set so the crate is publishable once vector-core ships to crates.io;
1919
# until then the `path` wins for local builds.
20-
vector-core = { path = "../vector-core", version = "0.3.0", default-features = false }
20+
vector-core = { path = "../vector-core", version = "0.4.0", default-features = false }
2121
nostr-sdk = "0.44.1"
2222
tokio = { version = "1.49", features = ["rt", "rt-multi-thread", "macros", "time"] }
2323
serde = { version = "1", features = ["derive"] }

src-tauri/Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)