Added support for multiple providers and wrote additional regression tests. Also fixed a bug with local synchronization with remote Git repositories when the CLI was just installed but the remote repo already exists with stuff in it.

2025-09-11 15:07:16 -06:00
parent 0f5c28a040
commit a8d959dac3
19 changed files with 1155 additions and 239 deletions
@@ -66,10 +66,16 @@ pub fn sync_and_push(opts: &SyncOpts<'_>) -> Result<()> {
    checkout_branch(&git, &repo_dir, branch)?;
    set_origin(&git, &repo_dir, remote_url)?;

-    stage_all(&git, &repo_dir)?;
-
+    // Always align local with remote before staging/committing. For a fresh
+    // repo where the remote already has content, we intentionally discard any
+    // local working tree changes and take the remote state to avoid merge
+    // conflicts on first sync.
    fetch_and_pull(&git, &repo_dir, branch)?;

+    // Stage and commit any subsequent local changes after aligning with remote
+    // so we don't merge uncommitted local state.
+    stage_all(&git, &repo_dir)?;
+
    commit_now(&git, &repo_dir, &commit_message)?;

    run_git(
@@ -228,17 +234,51 @@ fn stage_all(git: &Path, repo: &Path) -> Result<()> {
 }

 fn fetch_and_pull(git: &Path, repo: &Path, branch: &str) -> Result<()> {
-    run_git(git, repo, &["fetch", "origin", branch])
+    // Fetch all refs from origin (safe even if branch doesn't exist remotely)
+    run_git(git, repo, &["fetch", "origin", "--prune"]) 
        .with_context(|| "Failed to fetch changes from remote")?;
-    run_git(
-        git,
-        repo,
-        &["merge", "--ff-only", &format!("origin/{branch}")],
-    )
-    .with_context(|| "Failed to merge remote changes")?;
+
+    let origin_ref = format!("origin/{branch}");
+    let remote_has_branch = has_remote_branch(git, repo, branch);
+
+    // If the repo has no commits yet, prefer remote state and discard local
+    // if the remote branch exists. Otherwise, keep local state and allow an
+    // initial commit to be created and pushed.
+    if !has_head(git, repo) {
+        if remote_has_branch {
+            run_git(git, repo, &["checkout", "-f", "-B", branch, &origin_ref])
+                .with_context(|| "Failed to checkout remote branch over local state")?;
+            run_git(git, repo, &["reset", "--hard", &origin_ref])
+                .with_context(|| "Failed to hard reset to remote branch")?;
+            run_git(git, repo, &["clean", "-fd"]).with_context(|| "Failed to clean untracked files")?;
+        }
+        return Ok(());
+    }
+
+    // If we have local history and the remote branch exists, fast-forward.
+    if remote_has_branch {
+        run_git(
+            git,
+            repo,
+            &["merge", "--ff-only", &origin_ref],
+        )
+        .with_context(|| "Failed to merge remote changes")?;
+    }
    Ok(())
 }

+fn has_remote_branch(git: &Path, repo: &Path, branch: &str) -> bool {
+    Command::new(git)
+        .arg("-C")
+        .arg(repo)
+        .args(["show-ref", "--verify", "--quiet", &format!("refs/remotes/origin/{}", branch)])
+        .stdout(Stdio::null())
+        .stderr(Stdio::null())
+        .status()
+        .map(|s| s.success())
+        .unwrap_or(false)
+}
+
 fn has_head(git: &Path, repo: &Path) -> bool {
    Command::new(git)
        .arg("-C")
@@ -280,3 +320,54 @@ fn commit_now(git: &Path, repo: &Path, msg: &str) -> Result<()> {

    Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn sync_opts_validation_ok() {
+        let remote = Some("git@github.com:user/repo.git".to_string());
+        let branch = Some("main".to_string());
+        let opts = SyncOpts {
+            remote_url: &remote,
+            branch: &branch,
+            user_name: &None,
+            user_email: &None,
+            git_executable: &None,
+        };
+        assert!(opts.validate().is_ok());
+    }
+
+    #[test]
+    fn sync_opts_validation_missing_fields() {
+        let remote = None;
+        let branch = None;
+        let opts = SyncOpts {
+            remote_url: &remote,
+            branch: &branch,
+            user_name: &None,
+            user_email: &None,
+            git_executable: &None,
+        };
+        assert!(opts.validate().is_err());
+    }
+
+    #[test]
+    fn resolve_git_prefers_override_and_env() {
+        // Override path wins
+        let override_path = Some(PathBuf::from("/custom/git"));
+        let got = resolve_git(override_path.as_ref()).unwrap();
+        assert_eq!(got, PathBuf::from("/custom/git"));
+
+        // If no override, env var is used
+        unsafe {
+            env::set_var("GIT_EXECUTABLE", "/env/git");
+        }
+        let got_env = resolve_git(None).unwrap();
+        assert_eq!(got_env, PathBuf::from("/env/git"));
+        unsafe {
+            env::remove_var("GIT_EXECUTABLE");
+        }
+    }
+}
@@ -1,29 +1,30 @@
-use anyhow::{Context, anyhow, bail};
+use anyhow::{anyhow, bail, Context};
 use secrecy::{ExposeSecret, SecretString};
 use std::collections::HashMap;
 use std::fs;
 use zeroize::Zeroize;

-use crate::config::Config;
+use crate::config::ProviderConfig;
+use crate::providers::git_sync::{sync_and_push, SyncOpts};
 use crate::providers::SecretProvider;
-use crate::providers::git_sync::{SyncOpts, sync_and_push};
 use crate::{
    ARGON_M_COST_KIB, ARGON_P, ARGON_T_COST, HEADER, KDF, KEY_LEN, NONCE_LEN, SALT_LEN, VERSION,
 };
 use anyhow::Result;
 use argon2::{Algorithm, Argon2, Params, Version};
-use base64::{Engine as _, engine::general_purpose::STANDARD as B64};
+use base64::{engine::general_purpose::STANDARD as B64, Engine as _};
 use chacha20poly1305::aead::rand_core::RngCore;
 use chacha20poly1305::{
-    Key, XChaCha20Poly1305, XNonce,
    aead::{Aead, KeyInit, OsRng},
+    Key, XChaCha20Poly1305, XNonce,
 };
-use dialoguer::{Input, theme};
+use dialoguer::{theme, Input};
 use log::{debug, error};
 use serde::Deserialize;
 use theme::ColorfulTheme;
 use validator::Validate;

+/// Configuration for the local file-based provider.
 #[derive(Debug, Clone)]
 pub struct LocalProviderConfig {
    pub vault_path: String,
@@ -40,6 +41,25 @@ impl Default for LocalProviderConfig {
    }
 }

+/// File-based vault provider with optional Git sync.
+///
+/// This provider stores encrypted envelopes in a per-user configuration
+/// directory via `confy`. A password is obtained from a configured password
+/// file or via an interactive prompt.
+///
+/// Example
+/// ```no_run
+/// use gman::providers::local::LocalProvider;
+/// use gman::providers::SecretProvider;
+/// use gman::config::Config;
+///
+/// let provider = LocalProvider;
+/// let cfg = Config::default();
+/// // Will prompt for a password when reading/writing secrets unless a
+/// // password file is configured.
+/// // provider.set_secret(&cfg, "MY_SECRET", "value")?;
+/// # Ok::<(), anyhow::Error>(())
+/// ```
 #[derive(Debug, Clone, Copy, Default, Deserialize, PartialEq, Eq)]
 pub struct LocalProvider;

@@ -48,7 +68,7 @@ impl SecretProvider for LocalProvider {
        "LocalProvider"
    }

-    fn get_secret(&self, config: &Config, key: &str) -> Result<String> {
+    fn get_secret(&self, config: &ProviderConfig, key: &str) -> Result<String> {
        let vault: HashMap<String, String> = confy::load("gman", "vault").unwrap_or_default();
        let envelope = vault
            .get(key)
@@ -61,7 +81,7 @@ impl SecretProvider for LocalProvider {
        Ok(plaintext)
    }

-    fn set_secret(&self, config: &Config, key: &str, value: &str) -> Result<()> {
+    fn set_secret(&self, config: &ProviderConfig, key: &str, value: &str) -> Result<()> {
        let mut vault: HashMap<String, String> = confy::load("gman", "vault").unwrap_or_default();
        if vault.contains_key(key) {
            error!(
@@ -79,7 +99,7 @@ impl SecretProvider for LocalProvider {
        confy::store("gman", "vault", vault).with_context(|| "failed to save secret to the vault")
    }

-    fn update_secret(&self, config: &Config, key: &str, value: &str) -> Result<()> {
+    fn update_secret(&self, config: &ProviderConfig, key: &str, value: &str) -> Result<()> {
        let mut vault: HashMap<String, String> = confy::load("gman", "vault").unwrap_or_default();

        let password = get_password(config)?;
@@ -119,7 +139,7 @@ impl SecretProvider for LocalProvider {
        Ok(keys)
    }

-    fn sync(&self, config: &mut Config) -> Result<()> {
+    fn sync(&self, config: &mut ProviderConfig) -> Result<()> {
        let mut config_changed = false;

        if config.git_branch.is_none() {
@@ -139,9 +159,9 @@ impl SecretProvider for LocalProvider {
            let remote: String = Input::with_theme(&ColorfulTheme::default())
                .with_prompt("Enter remote git URL to sync with")
                .validate_with(|s: &String| {
-                    Config {
+                    ProviderConfig {
                        git_remote_url: Some(s.clone()),
-                        ..Config::default()
+                        ..ProviderConfig::default()
                    }
                    .validate()
                    .map(|_| ())
@@ -314,7 +334,7 @@ fn decrypt_string(password: &SecretString, envelope: &str) -> Result<String> {
    Ok(s)
 }

-fn get_password(config: &Config) -> Result<SecretString> {
+fn get_password(config: &ProviderConfig) -> Result<SecretString> {
    if let Some(password_file) = &config.password_file {
        let password = SecretString::new(
            fs::read_to_string(password_file)
@@ -333,10 +353,10 @@ fn get_password(config: &Config) -> Result<SecretString> {

 #[cfg(test)]
 mod tests {
-    use crate::derive_key;
-    use crate::providers::local::derive_key_with_params;
+    use super::*;
    use pretty_assertions::assert_eq;
-    use secrecy::SecretString;
+    use secrecy::{ExposeSecret, SecretString};
+    use tempfile::tempdir;

    #[test]
    fn test_derive_key() {
@@ -353,4 +373,26 @@ mod tests {
        let key = derive_key_with_params(&password, &salt, 10, 1, 1).unwrap();
        assert_eq!(key.as_slice().len(), 32);
    }
+
+    #[test]
+    fn crypto_roundtrip_local_impl() {
+        let pw = SecretString::new("pw".into());
+        let msg = "hello world";
+        let env = encrypt_string(&pw, msg).unwrap();
+        let out = decrypt_string(&pw, &env).unwrap();
+        assert_eq!(out, msg);
+    }
+
+    #[test]
+    fn get_password_reads_password_file() {
+        let dir = tempdir().unwrap();
+        let file = dir.path().join("pw.txt");
+        fs::write(&file, "secretpw\n").unwrap();
+        let cfg = ProviderConfig {
+            password_file: Some(file),
+            ..ProviderConfig::default()
+        };
+        let pw = get_password(&cfg).unwrap();
+        assert_eq!(pw.expose_secret(), "secretpw");
+    }
 }
@@ -1,19 +1,34 @@
+//! Secret provider trait and registry.
+//!
+//! Implementations provide storage/backends for secrets and a common
+//! interface used by the CLI.
+//!
+//! Selecting a provider from a string:
+//! ```
+//! use std::str::FromStr;
+//! use gman::providers::SupportedProvider;
+//!
+//! let p = SupportedProvider::from_str("local").unwrap();
+//! assert_eq!(p.to_string(), "local");
+//! ```
 mod git_sync;
 pub mod local;

-use crate::config::Config;
+use crate::config::ProviderConfig;
 use crate::providers::local::LocalProvider;
-use anyhow::{Result, anyhow};
+use anyhow::{anyhow, Result};
 use serde::Deserialize;
 use std::fmt::{Display, Formatter};
 use std::str::FromStr;
 use thiserror::Error;

+/// A secret storage backend capable of CRUD and sync, with optional
+/// update and listing
 pub trait SecretProvider {
    fn name(&self) -> &'static str;
-    fn get_secret(&self, config: &Config, key: &str) -> Result<String>;
-    fn set_secret(&self, config: &Config, key: &str, value: &str) -> Result<()>;
-    fn update_secret(&self, _config: &Config, _key: &str, _value: &str) -> Result<()> {
+    fn get_secret(&self, config: &ProviderConfig, key: &str) -> Result<String>;
+    fn set_secret(&self, config: &ProviderConfig, key: &str, value: &str) -> Result<()>;
+    fn update_secret(&self, _config: &ProviderConfig, _key: &str, _value: &str) -> Result<()> {
        Err(anyhow!(
            "update secret not supported for provider {}",
            self.name()
@@ -26,20 +41,28 @@ pub trait SecretProvider {
            self.name()
        ))
    }
-    fn sync(&self, config: &mut Config) -> Result<()>;
+    fn sync(&self, config: &mut ProviderConfig) -> Result<()>;
 }

+/// Errors when parsing a provider identifier.
 #[derive(Debug, Error)]
 pub enum ParseProviderError {
    #[error("unsupported provider '{0}'")]
    Unsupported(String),
 }

+/// Registry of built-in providers.
 #[derive(Debug, Clone, Copy, Deserialize, Eq, PartialEq)]
 pub enum SupportedProvider {
    Local(LocalProvider),
 }

+impl Default for SupportedProvider {
+    fn default() -> Self {
+        SupportedProvider::Local(LocalProvider)
+    }
+}
+
 impl FromStr for SupportedProvider {
    type Err = ParseProviderError;