Files
pixelpass/src/common/friends.rs
T
molluskandClaude Opus 4.8 6d0bf99076 fix(friends): five robustness bugs in the friends/control plane
Found in a bug audit of the just-merged friends-list feature. No crashes
or security holes, but five real state/correctness bugs:

- Host child dying on its own left the share campaign running, so it kept
  pushing a now-dead ticket to friends (retrying offline ones forever) and
  leaked share_status/met/share_code. The unexpected-exit path now captures
  the stderr error, then routes through the full stop_host() teardown
  (notably stop_share). (gui/mod.rs pump_host_events)

- on_friend_request downgraded an already-Accepted friend back to
  PendingIncoming when they re-sent a request (e.g. after losing their
  store). It now stays Accepted and re-confirms. (friends.rs)

- on_friend_accept advanced *any* known peer to Accepted, including a
  PendingIncoming one — a peer could mark itself accepted without the local
  user's consent. Now only a PendingOutgoing request we sent is honoured.
  (friends.rs)

- A ShareCode redelivered by an ACK-loss retry fired a duplicate desktop
  notification. push_notice now reports whether the code is new/changed and
  only then toasts. (gui/mod.rs)

- An inbound control message could be delayed up to IO_TIMEOUT on a degraded
  link because handle() awaited the sender's close before forwarding it.
  Forward to the UI first, then await close so the ACK still flushes.
  (control.rs)

Adds two friends-store transition tests (accept ignores a pending-incoming
peer; request doesn't downgrade an accepted friend). 47 gui / 8 headless
tests pass, clippy + fmt clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 15:10:22 -04:00

332 lines
13 KiB
Rust

//! Persistent friends store at `~/.config/pixelpass/friends.toml`.
//!
//! Kept in its own file rather than a `[friends]` section of `config.toml` so
//! the headless CLI — which never manages friends and would round-trip the
//! config without this knowledge — can't drop the list on a `--reconfigure`.
//! Same reasoning as the separate `identity.key`.
//!
//! A friend is identified by their stable control-plane [`EndpointId`] (the id
//! from [`super::endpoint::bind_control`]). `EndpointId` serialises as its
//! string form in TOML, so the file is human-readable and hand-editable.
use anyhow::{Context, Result};
use iroh::EndpointId;
use serde::{Deserialize, Serialize};
use std::fs;
use std::io::Write;
use std::path::PathBuf;
/// Where a friendship sits in the mutual-consent handshake.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum FriendState {
/// We've sent them a request and are waiting for them to accept.
PendingOutgoing,
/// They've requested us; waiting for the local user to accept or decline.
PendingIncoming,
/// Both sides have agreed — a real friend.
Accepted,
}
/// One entry in the friends list.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct Friend {
pub id: EndpointId,
/// Display name — seeded from the name the peer reported, locally editable.
pub name: String,
pub state: FriendState,
/// Whether the host auto-shares its session code with this friend. Toggled
/// on the host's share picker; persisted here so the choice survives a
/// restart. Defaults to `true` so a newly added friend is included (and an
/// older `friends.toml` without the field loads as share-with-all).
#[serde(default = "default_share")]
pub share: bool,
}
fn default_share() -> bool {
true
}
/// The persisted friends list. Serialises as a TOML array of tables
/// (`[[friends]]`).
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct FriendStore {
#[serde(default)]
pub friends: Vec<Friend>,
}
/// Returns `~/.config/pixelpass/friends.toml`. Shares the config directory with
/// [`super::config`]; the parent is created on save.
pub fn friends_path() -> Result<PathBuf> {
Ok(super::config::config_path()?
.parent()
.context("config path has no parent directory")?
.join("friends.toml"))
}
/// Load the store, or a default (empty) one if the file doesn't exist yet.
/// Parse errors bubble up so a hand-edit being debugged isn't silently
/// overwritten.
pub fn load() -> Result<FriendStore> {
let path = friends_path()?;
match fs::read_to_string(&path) {
Ok(s) => toml::from_str(&s).with_context(|| format!("failed to parse {}", path.display())),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(FriendStore::default()),
Err(e) => Err(e).with_context(|| format!("failed to read {}", path.display())),
}
}
impl FriendStore {
/// Atomic write via tempfile-in-same-dir + rename (mirrors
/// [`super::config::save`]).
pub fn save(&self) -> Result<()> {
let path = friends_path()?;
let parent = path
.parent()
.context("friends path has no parent directory")?;
fs::create_dir_all(parent)
.with_context(|| format!("failed to create {}", parent.display()))?;
let serialized = toml::to_string_pretty(self).context("failed to serialize friends")?;
let tmp = parent.join(format!(".friends.toml.tmp.{}", std::process::id()));
{
let mut f = fs::File::create(&tmp)
.with_context(|| format!("failed to create {}", tmp.display()))?;
f.write_all(serialized.as_bytes())
.with_context(|| format!("failed to write {}", tmp.display()))?;
f.sync_all().ok();
}
fs::rename(&tmp, &path)
.with_context(|| format!("failed to rename {} -> {}", tmp.display(), path.display()))?;
Ok(())
}
pub fn find(&self, id: &EndpointId) -> Option<&Friend> {
self.friends.iter().find(|f| &f.id == id)
}
pub fn find_mut(&mut self, id: &EndpointId) -> Option<&mut Friend> {
self.friends.iter_mut().find(|f| &f.id == id)
}
/// True iff this id is a fully-accepted friend — the gate the code-push
/// (Phase 4) and "is this a known friend?" checks use.
pub fn is_accepted(&self, id: &EndpointId) -> bool {
matches!(
self.find(id),
Some(Friend {
state: FriendState::Accepted,
..
})
)
}
/// Insert a new friend, or update an existing one's `name`/`state` in place.
/// Returns a mutable reference to the stored entry.
pub fn upsert(&mut self, id: EndpointId, name: String, state: FriendState) -> &mut Friend {
if let Some(idx) = self.friends.iter().position(|f| f.id == id) {
let f = &mut self.friends[idx];
f.name = name;
f.state = state;
f
} else {
self.friends.push(Friend {
id,
name,
state,
share: true,
});
self.friends.last_mut().expect("just pushed")
}
}
/// Remove a friend by id. Returns whether an entry was removed.
pub fn remove(&mut self, id: &EndpointId) -> bool {
let before = self.friends.len();
self.friends.retain(|f| &f.id != id);
self.friends.len() != before
}
/// Apply an inbound friend request. Returns `true` if the friendship is now
/// settled at [`Accepted`] and the caller should reply with a `FriendAccept`
/// — either because we'd already sent them a request (a mutual match) or
/// because they're an existing friend re-announcing (we never downgrade an
/// [`Accepted`] friend back to pending; a peer who lost their store and
/// re-adds us just gets re-confirmed). Otherwise it's recorded as
/// [`PendingIncoming`] for the user to act on and `false` is returned.
///
/// [`Accepted`]: FriendState::Accepted
/// [`PendingIncoming`]: FriendState::PendingIncoming
pub fn on_friend_request(&mut self, id: EndpointId, name: String) -> bool {
match self.find(&id).map(|f| f.state) {
Some(FriendState::PendingOutgoing | FriendState::Accepted) => {
self.upsert(id, name, FriendState::Accepted);
true
}
_ => {
self.upsert(id, name, FriendState::PendingIncoming);
false
}
}
}
/// Apply an inbound acceptance of a request we sent. Returns `true` only if
/// it advanced one of *our* outgoing requests to [`Accepted`]. An accept for
/// any other state is ignored: a stranger's, or one for a peer still in
/// [`PendingIncoming`] (their request, awaiting our decision) — honouring the
/// latter would let a peer mark itself accepted without the local user's
/// consent.
///
/// [`Accepted`]: FriendState::Accepted
/// [`PendingIncoming`]: FriendState::PendingIncoming
pub fn on_friend_accept(&mut self, id: EndpointId, name: String) -> bool {
if matches!(
self.find(&id).map(|f| f.state),
Some(FriendState::PendingOutgoing)
) {
self.upsert(id, name, FriendState::Accepted);
true
} else {
false
}
}
}
#[cfg(test)]
mod tests {
use super::*;
fn sample_id() -> EndpointId {
iroh::SecretKey::generate().public()
}
#[test]
fn round_trips_through_toml() {
let mut store = FriendStore::default();
store.upsert(sample_id(), "Alice".into(), FriendState::Accepted);
store.upsert(sample_id(), "Bob".into(), FriendState::PendingIncoming);
let toml = toml::to_string_pretty(&store).unwrap();
let back: FriendStore = toml::from_str(&toml).unwrap();
assert_eq!(back.friends, store.friends);
}
#[test]
fn new_friends_default_to_shared_and_survive_round_trip() {
let mut store = FriendStore::default();
let id = sample_id();
store.upsert(id, "Alice".into(), FriendState::Accepted);
assert!(store.find(&id).unwrap().share, "new friends start shared");
// An older friends.toml predating the field loads as share-with-all.
let toml = format!("[[friends]]\nid = \"{id}\"\nname = \"Legacy\"\nstate = \"accepted\"\n");
let back: FriendStore = toml::from_str(&toml).unwrap();
assert!(back.friends[0].share);
}
#[test]
fn upsert_preserves_share_across_refresh() {
let mut store = FriendStore::default();
let id = sample_id();
store.upsert(id, "Alice".into(), FriendState::Accepted);
store.find_mut(&id).unwrap().share = false;
// A later name/presence refresh re-upserts the same peer; the share
// choice must not be reset by it.
store.upsert(id, "Alice (new name)".into(), FriendState::Accepted);
assert!(!store.find(&id).unwrap().share);
}
#[test]
fn upsert_updates_in_place() {
let mut store = FriendStore::default();
let id = sample_id();
store.upsert(id, "Old".into(), FriendState::PendingOutgoing);
store.upsert(id, "New".into(), FriendState::Accepted);
assert_eq!(store.friends.len(), 1);
let f = store.find(&id).unwrap();
assert_eq!(f.name, "New");
assert_eq!(f.state, FriendState::Accepted);
}
#[test]
fn is_accepted_only_for_accepted_state() {
let mut store = FriendStore::default();
let pending = sample_id();
let friend = sample_id();
store.upsert(pending, "P".into(), FriendState::PendingOutgoing);
store.upsert(friend, "F".into(), FriendState::Accepted);
assert!(!store.is_accepted(&pending));
assert!(store.is_accepted(&friend));
assert!(!store.is_accepted(&sample_id()));
}
#[test]
fn remove_reports_whether_present() {
let mut store = FriendStore::default();
let id = sample_id();
store.upsert(id, "X".into(), FriendState::Accepted);
assert!(store.remove(&id));
assert!(!store.remove(&id));
assert!(store.friends.is_empty());
}
#[test]
fn incoming_request_from_stranger_is_pending() {
let mut store = FriendStore::default();
let id = sample_id();
let mutual = store.on_friend_request(id, "Stranger".into());
assert!(!mutual);
assert_eq!(store.find(&id).unwrap().state, FriendState::PendingIncoming);
}
#[test]
fn incoming_request_matching_our_outgoing_is_mutual() {
let mut store = FriendStore::default();
let id = sample_id();
// We asked them first…
store.upsert(id, "Pal".into(), FriendState::PendingOutgoing);
// …then their request arrives — that's a mutual match.
let mutual = store.on_friend_request(id, "Pal".into());
assert!(mutual);
assert_eq!(store.find(&id).unwrap().state, FriendState::Accepted);
}
#[test]
fn accept_advances_known_peer_only() {
let mut store = FriendStore::default();
let known = sample_id();
store.upsert(known, "Known".into(), FriendState::PendingOutgoing);
assert!(store.on_friend_accept(known, "Known".into()));
assert_eq!(store.find(&known).unwrap().state, FriendState::Accepted);
// An accept from someone we never asked is ignored.
let stranger = sample_id();
assert!(!store.on_friend_accept(stranger, "Nope".into()));
assert!(store.find(&stranger).is_none());
}
#[test]
fn accept_does_not_advance_a_pending_incoming_peer() {
// They asked us and we haven't decided yet; an unsolicited FriendAccept
// from them must not auto-accept on our behalf (consent bypass).
let mut store = FriendStore::default();
let id = sample_id();
store.upsert(id, "Theirs".into(), FriendState::PendingIncoming);
assert!(!store.on_friend_accept(id, "Theirs".into()));
assert_eq!(store.find(&id).unwrap().state, FriendState::PendingIncoming);
}
#[test]
fn request_does_not_downgrade_an_accepted_friend() {
// A current friend re-sending a request (e.g. after losing their store)
// must stay accepted; the call signals a re-confirm rather than a
// downgrade to pending.
let mut store = FriendStore::default();
let id = sample_id();
store.upsert(id, "Pal".into(), FriendState::Accepted);
let settled = store.on_friend_request(id, "Pal (reinstalled)".into());
assert!(settled);
assert_eq!(store.find(&id).unwrap().state, FriendState::Accepted);
assert_eq!(store.find(&id).unwrap().name, "Pal (reinstalled)");
}
}