radicle: Prevent `RAD_SOCKET` from polluting tests

The value of `RAD_SOCKET` influences the outcome of `cargo test`.

Users that set this environment variable to, e.g. point `rad` at
a socket at a custom location, were surprised that execution of
tests would interact with their node.

Split `Home::socket` into two functions. `Home::socket_default` is
"pure" and only dependent on the path represented by `Home`. On the
other hand, `Home::socket_from_env` interprets the environment.

Most users will want to use `Home::socket_from_env`.

crates/radicle-cli/src/commands/clone.rs

@@ -30,7 +30,7 @@ pub use args::Args;
 
 pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
-    let mut node = radicle::Node::new(profile.socket());
+    let mut node = radicle::Node::new(profile.socket_from_env());
 
     if !node.is_running() {
         anyhow::bail!(

crates/radicle-cli/src/commands/follow.rs

@@ -11,7 +11,7 @@ use args::Operation;
 
 pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
-    let mut node = radicle::Node::new(profile.socket());
+    let mut node = radicle::Node::new(profile.socket_from_env());
 
     match Operation::from(args) {
         Operation::Follow { nid, alias, .. } => follow(nid, alias, &mut node, &profile)?,

crates/radicle-cli/src/commands/init.rs

@@ -136,7 +136,7 @@ pub fn init(repo: git::Repository, args: Args, profile: &profile::Profile) -> an
     };
 
     let signer = term::signer(profile)?;
-    let mut node = radicle::Node::new(profile.socket());
+    let mut node = radicle::Node::new(profile.socket_from_env());
     let mut spinner = term::spinner("Initializing...");
     let mut push_cmd = String::from("git push");
 

crates/radicle-cli/src/commands/issue.rs

@@ -219,7 +219,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     }
 
     if announce {
-        let mut node = Node::new(profile.socket());
+        let mut node = Node::new(profile.socket_from_env());
         node::announce(
             &repo,
             node::SyncSettings::default(),

crates/radicle-cli/src/commands/node.rs

@@ -23,7 +23,7 @@ use args::{Addr, Command};
 
 pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
-    let mut node = Node::new(profile.socket());
+    let mut node = Node::new(profile.socket_from_env());
 
     let command = args.command.unwrap_or_default();
 

crates/radicle-cli/src/commands/patch.rs

@@ -314,7 +314,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     }
 
     if announce {
-        let mut node = Node::new(profile.socket());
+        let mut node = Node::new(profile.socket_from_env());
         node::announce(
             &repository,
             node::SyncSettings::default(),

crates/radicle-cli/src/commands/publish.rs

@@ -63,7 +63,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
         }
         anyhow::bail!("fatal: repository storage is corrupt");
     }
-    let mut node = radicle::Node::new(profile.socket());
+    let mut node = radicle::Node::new(profile.socket_from_env());
     let spinner = term::spinner("Updating inventory..");
 
     // The repository is now part of our inventory.

crates/radicle-cli/src/commands/remote/add.rs

@@ -23,7 +23,7 @@ pub fn run(
     sync: bool,
 ) -> anyhow::Result<()> {
     if sync {
-        let mut node = radicle::Node::new(profile.socket());
+        let mut node = radicle::Node::new(profile.socket_from_env());
 
         if !profile.policies()?.is_following(nid)? {
             let alias = name.as_ref().and_then(|n| Alias::from_str(n.as_str()).ok());

crates/radicle-cli/src/commands/seed.rs

@@ -13,7 +13,7 @@ pub use args::Args;
 
 pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
-    let mut node = radicle::Node::new(profile.socket());
+    let mut node = radicle::Node::new(profile.socket_from_env());
 
     match args::Operation::from(args) {
         args::Operation::List => seeding(&profile)?,

crates/radicle-cli/src/commands/self.rs

@@ -28,7 +28,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     } else if args.nid {
         crate::warning::deprecated("rad self --nid", "rad node status --only nid");
         term::print(
-            Node::new(profile.socket())
+            Node::new(profile.socket_from_env())
                 .nid()
                 .ok()
                 .unwrap_or_else(|| *profile.id()),
@@ -54,7 +54,7 @@ fn all(profile: &Profile) -> anyhow::Result<()> {
         term::format::tertiary(did).into(),
     ]);
 
-    let socket = profile.socket();
+    let socket = profile.socket_from_env();
     let node = if Node::new(&socket).is_running() {
         term::format::positive(format!("running ({})", socket.display()))
     } else {

crates/radicle-cli/src/commands/sync.rs

@@ -30,7 +30,7 @@ use args::{Command, SortBy, SyncDirection, SyncMode};
 
 pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
-    let mut node = radicle::Node::new(profile.socket());
+    let mut node = radicle::Node::new(profile.socket_from_env());
     if !node.is_running() {
         anyhow::bail!(
             "to sync a repository, your node must be running. To start it, run `rad node start`"

crates/radicle-cli/src/commands/unfollow.rs

@@ -8,7 +8,7 @@ pub use args::Args;
 
 pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
-    let mut node = radicle::Node::new(profile.socket());
+    let mut node = radicle::Node::new(profile.socket_from_env());
     let nid = args.nid;
 
     let unfollowed = match node.unfollow(nid) {

crates/radicle-cli/src/commands/unseed.rs

@@ -8,7 +8,7 @@ pub use args::Args;
 
 pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
-    let mut node = radicle::Node::new(profile.socket());
+    let mut node = radicle::Node::new(profile.socket_from_env());
 
     for rid in args.rids {
         delete(rid, &mut node, &profile)?;

crates/radicle-node/src/main.rs

@@ -329,11 +329,12 @@ fn execute(options: Options) -> Result<(), ExecutionError> {
         signals
     };
 
+    let socket = home.socket_from_env();
     if options.force {
         log::debug!(target: "node", "Removing existing control socket..");
-        std::fs::remove_file(home.socket()).ok();
+        std::fs::remove_file(&socket).ok();
     }
-    Runtime::init(home, config.node, listen, signals, signer)?.run()?;
+    Runtime::init(home, config.node, socket, listen, signals, signer)?.run()?;
 
     Ok(())
 }

crates/radicle-node/src/runtime.rs

@@ -127,6 +127,7 @@ impl Runtime {
     pub fn init<G>(
         home: Home,
         config: radicle::node::Config,
+        socket: PathBuf,
         listen: Vec<net::SocketAddr>,
         signals: chan::Receiver<Signal>,
         signer: Device<G>,
@@ -233,7 +234,7 @@ impl Runtime {
             wire.listen(listener);
         }
         let reactor = Reactor::new(wire, thread::name(&id, "service"))?;
-        let handle = Handle::new(home.clone(), reactor.controller(), emitter);
+        let handle = Handle::new(home.clone(), socket.clone(), reactor.controller(), emitter);
 
         let nid = *signer.public_key();
         let fetch = worker::FetchConfig {
@@ -255,7 +256,7 @@ impl Runtime {
                 policies_db: home.node().join(node::POLICIES_DB_FILE),
             },
         )?;
-        let control = Self::bind(home.socket())?;
+        let control = Self::bind(socket)?;
 
         Ok(Runtime {
             id,

crates/radicle-node/src/runtime/handle.rs

@@ -1,5 +1,6 @@
 use std::collections::HashSet;
 use std::net;
+use std::path::PathBuf;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::{fmt, io, time};
@@ -71,6 +72,10 @@ impl<T> From<chan::SendError<T>> for Error {
 
 pub struct Handle {
     pub(crate) home: Home,
+
+    /// Path to the control socket in use. Required for shutdown.
+    pub(crate) socket: PathBuf,
+
     pub(crate) controller: reactor::Controller,
 
     /// Whether a shutdown was initiated or not. Prevents attempting to shutdown twice.
@@ -96,6 +101,7 @@ impl Clone for Handle {
     fn clone(&self) -> Self {
         Self {
             home: self.home.clone(),
+            socket: self.socket.clone(),
             controller: self.controller.clone(),
             shutdown: self.shutdown.clone(),
             emitter: self.emitter.clone(),
@@ -104,9 +110,15 @@ impl Clone for Handle {
 }
 
 impl Handle {
-    pub fn new(home: Home, controller: reactor::Controller, emitter: Emitter<Event>) -> Self {
+    pub fn new(
+        home: Home,
+        socket: PathBuf,
+        controller: reactor::Controller,
+        emitter: Emitter<Event>,
+    ) -> Self {
         Self {
             home,
+            socket,
             controller,
             shutdown: Arc::default(),
             emitter,
@@ -352,7 +364,7 @@ impl radicle::node::Handle for Handle {
         // Send a shutdown request to our own control socket. This is the only way to kill the
         // control thread gracefully. Since the control thread may have called this function,
         // the control socket may already be disconnected. Ignore errors.
-        UnixStream::connect(self.home.socket())
+        UnixStream::connect(self.socket)
             .and_then(|sock| Command::Shutdown.to_writer(sock))
             .ok();
 

crates/radicle-node/src/test/node.rs

@@ -478,6 +478,7 @@ impl<G: cyphernet::Ecdh<Pk = NodeId> + Signer<Signature> + Clone + Debug> Node<G
         let rt = Runtime::init(
             self.home.clone(),
             self.config,
+            self.home.socket_default(),
             listen,
             signals,
             self.signer.clone(),

crates/radicle-remote-helper/src/service.rs

@@ -65,7 +65,7 @@ pub(super) struct RealNodeSession {
 impl RealNodeSession {
     pub(super) fn new(profile: &Profile) -> Self {
         Self {
-            node: radicle::Node::new(profile.socket()),
+            node: radicle::Node::new(profile.socket_from_env()),
         }
     }
 }

crates/radicle/CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- To obtain the location of the control socket, do not use `Home::socket`
+  (which was removed), but instead choose `Home::socket_from_env` or
+  `Home::socket_default` depending on your use-case.
+
 ### Removed
 
 ### Security

crates/radicle/src/profile.rs

@@ -667,12 +667,25 @@ impl Home {
         self.path.join("cobs")
     }
 
-    pub fn socket(&self) -> PathBuf {
-        const DEFAULT_SOCKET_NAME: &str = "control.sock";
-
+    /// The location of the control socket of the node.
+    /// If the environment variable with name [`env::RAD_SOCKET`] is set,
+    /// its value is used.
+    /// Otherwise, the default socket name, which is relative to this
+    /// [`Home`], is used (see [`Self::socket_default`]).
+    pub fn socket_from_env(&self) -> PathBuf {
         env::var_os(env::RAD_SOCKET)
             .map(PathBuf::from)
-            .unwrap_or_else(|| self.node().join(DEFAULT_SOCKET_NAME))
+            .unwrap_or_else(|| self.socket_default())
+    }
+
+    /// The default location of the control socket of the node.
+    /// The returned value only depends on `self`, and not on
+    /// any environment variables.
+    ///
+    /// See also [`Self::socket_from_env`].
+    pub fn socket_default(&self) -> PathBuf {
+        const DEFAULT_SOCKET_NAME: &str = "control.sock";
+        self.node().join(DEFAULT_SOCKET_NAME)
     }
 
     /// Return a read-write handle to the notifications database.