From 80c1bba76f432e70d6e766fe750bea70554cfe86 Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Sat, 21 Mar 2026 15:03:00 +0100 Subject: [PATCH] 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 | 2 +- crates/radicle-cli/src/commands/follow.rs | 2 +- crates/radicle-cli/src/commands/init.rs | 2 +- crates/radicle-cli/src/commands/issue.rs | 2 +- crates/radicle-cli/src/commands/node.rs | 2 +- crates/radicle-cli/src/commands/patch.rs | 2 +- crates/radicle-cli/src/commands/publish.rs | 2 +- crates/radicle-cli/src/commands/remote/add.rs | 2 +- crates/radicle-cli/src/commands/seed.rs | 2 +- crates/radicle-cli/src/commands/self.rs | 4 ++-- crates/radicle-cli/src/commands/sync.rs | 2 +- crates/radicle-cli/src/commands/unfollow.rs | 2 +- crates/radicle-cli/src/commands/unseed.rs | 2 +- crates/radicle-node/src/main.rs | 5 +++-- crates/radicle-node/src/runtime.rs | 5 +++-- crates/radicle-node/src/runtime/handle.rs | 16 ++++++++++++-- crates/radicle-node/src/test/node.rs | 1 + crates/radicle-remote-helper/src/service.rs | 2 +- crates/radicle/CHANGELOG.md | 4 ++++ crates/radicle/src/profile.rs | 21 +++++++++++++++---- 20 files changed, 57 insertions(+), 25 deletions(-) diff --git a/crates/radicle-cli/src/commands/clone.rs b/crates/radicle-cli/src/commands/clone.rs index 5989028c..e0adfdce 100644 --- a/crates/radicle-cli/src/commands/clone.rs +++ b/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!( diff --git a/crates/radicle-cli/src/commands/follow.rs b/crates/radicle-cli/src/commands/follow.rs index 27dff43b..194046b8 100644 --- a/crates/radicle-cli/src/commands/follow.rs +++ b/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)?, diff --git a/crates/radicle-cli/src/commands/init.rs b/crates/radicle-cli/src/commands/init.rs index 997bcf65..de22c245 100644 --- a/crates/radicle-cli/src/commands/init.rs +++ b/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"); diff --git a/crates/radicle-cli/src/commands/issue.rs b/crates/radicle-cli/src/commands/issue.rs index d18dd484..6717ccd6 100644 --- a/crates/radicle-cli/src/commands/issue.rs +++ b/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(), diff --git a/crates/radicle-cli/src/commands/node.rs b/crates/radicle-cli/src/commands/node.rs index f7f2f722..3e8ffa86 100644 --- a/crates/radicle-cli/src/commands/node.rs +++ b/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(); diff --git a/crates/radicle-cli/src/commands/patch.rs b/crates/radicle-cli/src/commands/patch.rs index c4a17907..25136f17 100644 --- a/crates/radicle-cli/src/commands/patch.rs +++ b/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(), diff --git a/crates/radicle-cli/src/commands/publish.rs b/crates/radicle-cli/src/commands/publish.rs index 39e9e3fa..48406b94 100644 --- a/crates/radicle-cli/src/commands/publish.rs +++ b/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. diff --git a/crates/radicle-cli/src/commands/remote/add.rs b/crates/radicle-cli/src/commands/remote/add.rs index f130e669..f4f24609 100644 --- a/crates/radicle-cli/src/commands/remote/add.rs +++ b/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()); diff --git a/crates/radicle-cli/src/commands/seed.rs b/crates/radicle-cli/src/commands/seed.rs index fa2799fb..82cfb90d 100644 --- a/crates/radicle-cli/src/commands/seed.rs +++ b/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)?, diff --git a/crates/radicle-cli/src/commands/self.rs b/crates/radicle-cli/src/commands/self.rs index ec3d3c2a..96583f5f 100644 --- a/crates/radicle-cli/src/commands/self.rs +++ b/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 { diff --git a/crates/radicle-cli/src/commands/sync.rs b/crates/radicle-cli/src/commands/sync.rs index b0806fcd..ac1dd937 100644 --- a/crates/radicle-cli/src/commands/sync.rs +++ b/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`" diff --git a/crates/radicle-cli/src/commands/unfollow.rs b/crates/radicle-cli/src/commands/unfollow.rs index 9e18555d..a4214a41 100644 --- a/crates/radicle-cli/src/commands/unfollow.rs +++ b/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) { diff --git a/crates/radicle-cli/src/commands/unseed.rs b/crates/radicle-cli/src/commands/unseed.rs index df9fdfd4..04a444bc 100644 --- a/crates/radicle-cli/src/commands/unseed.rs +++ b/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)?; diff --git a/crates/radicle-node/src/main.rs b/crates/radicle-node/src/main.rs index 61041804..60d4fca8 100644 --- a/crates/radicle-node/src/main.rs +++ b/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(()) } diff --git a/crates/radicle-node/src/runtime.rs b/crates/radicle-node/src/runtime.rs index 7cd7f55f..322ad521 100644 --- a/crates/radicle-node/src/runtime.rs +++ b/crates/radicle-node/src/runtime.rs @@ -127,6 +127,7 @@ impl Runtime { pub fn init( home: Home, config: radicle::node::Config, + socket: PathBuf, listen: Vec, signals: chan::Receiver, signer: Device, @@ -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, diff --git a/crates/radicle-node/src/runtime/handle.rs b/crates/radicle-node/src/runtime/handle.rs index dcd07219..01e182e6 100644 --- a/crates/radicle-node/src/runtime/handle.rs +++ b/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 From> 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) -> Self { + pub fn new( + home: Home, + socket: PathBuf, + controller: reactor::Controller, + emitter: Emitter, + ) -> 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(); diff --git a/crates/radicle-node/src/test/node.rs b/crates/radicle-node/src/test/node.rs index 52d7f3f7..8cc216d2 100644 --- a/crates/radicle-node/src/test/node.rs +++ b/crates/radicle-node/src/test/node.rs @@ -478,6 +478,7 @@ impl + Signer + Clone + Debug> Node Self { Self { - node: radicle::Node::new(profile.socket()), + node: radicle::Node::new(profile.socket_from_env()), } } } diff --git a/crates/radicle/CHANGELOG.md b/crates/radicle/CHANGELOG.md index 28daee83..77223d8b 100644 --- a/crates/radicle/CHANGELOG.md +++ b/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 diff --git a/crates/radicle/src/profile.rs b/crates/radicle/src/profile.rs index 8b90c1eb..26698815 100644 --- a/crates/radicle/src/profile.rs +++ b/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.