fix: improve broken symlink error handling (#2589)

Check atuin setting paths (eg. `db_path`) for broken symlinks on
initialization and disable all shell hooks + print error message.

sqlite doesn't create db files even with `.create_if_missing` when the
db files are a broken symlink. This would cause sqlite to error and
atuin to panic on every single keypress.

Also improves related error handling when calling atuin client commands
directly.

8 files changed, 99 insertions, 29 deletions

diff --git a/crates/atuin-client/src/database.rs b/crates/atuin-client/src/database.rs
index 5bdbb75c..b64ff4ce 100644
--- a/crates/atuin-client/src/database.rs
+++ b/crates/atuin-client/src/database.rs
@@ -130,8 +130,12 @@ impl Sqlite {
         let path = path.as_ref();
         debug!("opening sqlite database at {:?}", path);
 
-        let create = !path.exists();
-        if create {
+        if utils::broken_symlink(path) {
+            eprintln!("Atuin: Sqlite db path ({path:?}) is a broken symlink. Unable to read or create replacement.");
+            std::process::exit(1);
+        }
+
+        if !path.exists() {
             if let Some(dir) = path.parent() {
                 fs::create_dir_all(dir)?;
             }
@@ -150,7 +154,6 @@ impl Sqlite {
             .await?;
 
         Self::setup_db(&pool).await?;
-
         Ok(Self { pool })
     }
 
diff --git a/crates/atuin-client/src/record/sqlite_store.rs b/crates/atuin-client/src/record/sqlite_store.rs
index 2937dbd7..c42476d4 100644
--- a/crates/atuin-client/src/record/sqlite_store.rs
+++ b/crates/atuin-client/src/record/sqlite_store.rs
@@ -17,6 +17,7 @@ use sqlx::{
 use atuin_common::record::{
     EncryptedData, Host, HostId, Record, RecordId, RecordIdx, RecordStatus,
 };
+use atuin_common::utils;
 use uuid::Uuid;
 
 use super::encryption::PASETO_V4;
@@ -33,8 +34,12 @@ impl SqliteStore {
 
         debug!("opening sqlite database at {:?}", path);
 
-        let create = !path.exists();
-        if create {
+        if utils::broken_symlink(path) {
+            eprintln!("Atuin: Sqlite db path ({path:?}) is a broken symlink. Unable to read or create replacement.");
+            std::process::exit(1);
+        }
+
+        if !path.exists() {
             if let Some(dir) = path.parent() {
                 fs::create_dir_all(dir)?;
             }
diff --git a/crates/atuin-client/src/settings.rs b/crates/atuin-client/src/settings.rs
index c01281c7..a1b0724f 100644
--- a/crates/atuin-client/src/settings.rs
+++ b/crates/atuin-client/src/settings.rs
@@ -3,6 +3,7 @@ use std::{
 };
 
 use atuin_common::record::HostId;
+use atuin_common::utils;
 use clap::ValueEnum;
 use config::{
     builder::DefaultState, Config, ConfigBuilder, Environment, File as ConfigFile, FileFormat,
@@ -852,24 +853,33 @@ impl Settings {
             .map_err(|e| eyre!("failed to deserialize: {}", e))?;
 
         // all paths should be expanded
-        let db_path = settings.db_path;
-        let db_path = shellexpand::full(&db_path)?;
-        settings.db_path = db_path.to_string();
-
-        let key_path = settings.key_path;
-        let key_path = shellexpand::full(&key_path)?;
-        settings.key_path = key_path.to_string();
-
-        let session_path = settings.session_path;
-        let session_path = shellexpand::full(&session_path)?;
-        settings.session_path = session_path.to_string();
+        settings.db_path = Self::expand_path(settings.db_path)?;
+        settings.record_store_path = Self::expand_path(settings.record_store_path)?;
+        settings.key_path = Self::expand_path(settings.key_path)?;
+        settings.session_path = Self::expand_path(settings.session_path)?;
 
         Ok(settings)
     }
 
+    fn expand_path(path: String) -> Result<String> {
+        shellexpand::full(&path)
+            .map(|p| p.to_string())
+            .map_err(|e| eyre!("failed to expand path: {}", e))
+    }
+
     pub fn example_config() -> &'static str {
         EXAMPLE_CONFIG
     }
+
+    pub fn paths_ok(&self) -> bool {
+        let paths = [
+            &self.db_path,
+            &self.record_store_path,
+            &self.key_path,
+            &self.session_path,
+        ];
+        paths.iter().all(|p| !utils::broken_symlink(p))
+    }
 }
 
 impl Default for Settings {
diff --git a/crates/atuin-common/src/utils.rs b/crates/atuin-common/src/utils.rs
index fbf2ef82..d495de36 100644
--- a/crates/atuin-common/src/utils.rs
+++ b/crates/atuin-common/src/utils.rs
@@ -113,6 +113,11 @@ pub fn get_current_dir() -> String {
     }
 }
 
+pub fn broken_symlink<P: Into<PathBuf>>(path: P) -> bool {
+    let path = path.into();
+    path.is_symlink() && !path.exists()
+}
+
 pub fn is_zsh() -> bool {
     // only set on zsh
     env::var("ATUIN_SHELL_ZSH").is_ok()
diff --git a/crates/atuin/src/command/client.rs b/crates/atuin/src/command/client.rs
index 2637b691..723bb974 100644
--- a/crates/atuin/src/command/client.rs
+++ b/crates/atuin/src/command/client.rs
@@ -129,6 +129,7 @@ impl Cmd {
         match self {
             Self::History(history) => return history.run(&settings).await,
             Self::Init(init) => return init.run(&settings).await,
+            Self::Doctor => return doctor::run(&settings).await,
             _ => {}
         }
 
@@ -163,8 +164,6 @@ impl Cmd {
                 Ok(())
             }
 
-            Self::Doctor => doctor::run(&settings).await,
-
             Self::DefaultConfig => {
                 default_config::run();
                 Ok(())
@@ -175,7 +174,7 @@ impl Cmd {
             #[cfg(feature = "daemon")]
             Self::Daemon => daemon::run(settings, sqlite_store, db).await,
 
-            _ => unimplemented!(),
+            Self::History(_) | Self::Init(_) | Self::Doctor => unreachable!(),
         }
     }
 }
diff --git a/crates/atuin/src/command/client/doctor.rs b/crates/atuin/src/command/client/doctor.rs
index fcd3449e..8203de82 100644
--- a/crates/atuin/src/command/client/doctor.rs
+++ b/crates/atuin/src/command/client/doctor.rs
@@ -4,13 +4,14 @@ use std::{env, path::PathBuf, str::FromStr};
 use atuin_client::database::Sqlite;
 use atuin_client::settings::Settings;
 use atuin_common::shell::{shell_name, Shell};
+use atuin_common::utils;
 use colored::Colorize;
 use eyre::Result;
-use serde::{Deserialize, Serialize};
+use serde::Serialize;
 
 use sysinfo::{get_current_pid, Disks, System};
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Serialize)]
 struct ShellInfo {
     pub name: String,
 
@@ -196,13 +197,13 @@ impl ShellInfo {
     }
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Serialize)]
 struct DiskInfo {
     pub name: String,
     pub filesystem: String,
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Serialize)]
 struct SystemInfo {
     pub os: String,
 
@@ -233,7 +234,7 @@ impl SystemInfo {
     }
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Serialize)]
 struct SyncInfo {
     /// Whether the main Atuin sync server is in use
     /// I'm just calling it Atuin Cloud for lack of a better name atm
@@ -255,7 +256,43 @@ impl SyncInfo {
     }
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug)]
+struct SettingPaths {
+    db: String,
+    record_store: String,
+    key: String,
+    session: String,
+}
+
+impl SettingPaths {
+    pub fn new(settings: &Settings) -> Self {
+        Self {
+            db: settings.db_path.clone(),
+            record_store: settings.record_store_path.clone(),
+            key: settings.key_path.clone(),
+            session: settings.session_path.clone(),
+        }
+    }
+
+    pub fn verify(&self) {
+        let paths = vec![
+            ("ATUIN_DB_PATH", &self.db),
+            ("ATUIN_RECORD_STORE", &self.record_store),
+            ("ATUIN_KEY", &self.key),
+            ("ATUIN_SESSION", &self.session),
+        ];
+
+        for (path_env_var, path) in paths {
+            if utils::broken_symlink(path) {
+                eprintln!(
+                    "{path} (${path_env_var}) is a broken symlink. This may cause issues with Atuin."
+                );
+            }
+        }
+    }
+}
+
+#[derive(Debug, Serialize)]
 struct AtuinInfo {
     pub version: String,
 
@@ -264,6 +301,9 @@ struct AtuinInfo {
     pub sync: Option<SyncInfo>,
 
     pub sqlite_version: String,
+
+    #[serde(skip)] // probably unnecessary to expose this
+    pub setting_paths: SettingPaths,
 }
 
 impl AtuinInfo {
@@ -289,11 +329,12 @@ impl AtuinInfo {
             version: crate::VERSION.to_string(),
             sync,
             sqlite_version,
+            setting_paths: SettingPaths::new(settings),
         }
     }
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Serialize)]
 struct DoctorDump {
     pub atuin: AtuinInfo,
     pub shell: ShellInfo,
@@ -322,6 +363,8 @@ fn checks(info: &DoctorDump) {
         println!("{zfs_error}");
     }
 
+    info.atuin.setting_paths.verify();
+
     // Shell
     if info.shell.name == "bash" {
         if !info
diff --git a/crates/atuin/src/command/client/init.rs b/crates/atuin/src/command/client/init.rs
index 8238a69b..7f8b2d32 100644
--- a/crates/atuin/src/command/client/init.rs
+++ b/crates/atuin/src/command/client/init.rs
@@ -159,6 +159,11 @@ $env.config = (
     }
 
     pub async fn run(self, settings: &Settings) -> Result<()> {
+        if !settings.paths_ok() {
+            eprintln!("Atuin settings paths are broken. Disabling atuin shell hooks. Run `atuin doctor` to diagnose.");
+            return Ok(());
+        }
+
         if settings.dotfiles.enabled {
             self.dotfiles_init(settings).await?;
         } else {
diff --git a/crates/atuin/src/command/client/search.rs b/crates/atuin/src/command/client/search.rs
index b6504bd9..4c24620b 100644
--- a/crates/atuin/src/command/client/search.rs
+++ b/crates/atuin/src/command/client/search.rs
@@ -160,17 +160,17 @@ impl Cmd {
             // when running the equivalent search, but deleting those entries that are
             // displayed with the search would leave any duplicates of those lines which may
             // or may not have been intended to be deleted.
-            println!("\"--limit\" is not compatible with deletion.");
+            eprintln!("\"--limit\" is not compatible with deletion.");
             return Ok(());
         }
 
         if self.delete && query.is_empty() {
-            println!("Please specify a query to match the items you wish to delete. If you wish to delete all history, pass --delete-it-all");
+            eprintln!("Please specify a query to match the items you wish to delete. If you wish to delete all history, pass --delete-it-all");
             return Ok(());
         }
 
         if self.delete_it_all && !query.is_empty() {
-            println!(
+            eprintln!(
                 "--delete-it-all will delete ALL of your history! It does not require a query."
             );
             return Ok(());