feat: update auth of interal repos when changed in config

Closes kemitix/git-next#100

.cargo/config.toml

@@ -13,5 +13,5 @@ debug = 0
 strip = "debuginfo"
 
 [env]
-RUST_LOG = "hyper=warn,debug"
+RUST_LOG = "hyper=warn,git_url_parse=warn,debug"
 RUSTFLAGS = "--cfg tokio_unstable"

Cargo.toml

@@ -61,6 +61,7 @@ gix = { version = "0.63", features = [
   "blocking-http-transport-reqwest-rust-tls",
 ] }
 async-trait = "0.1"
+git-url-parse = "0.4"
 
 # fs/network
 kxio = { version = "1.2" }
@@ -91,6 +92,7 @@ derive_more = { version = "1.0.0-beta.6", features = [
 ] }
 derive-with = "0.5"
 thiserror = "1.0"
+pike = "0.1"
 
 # file watcher
 inotify = "0.10"

crates/config/Cargo.toml

@@ -23,6 +23,10 @@ toml = { workspace = true }
 # Secrets and Password
 secrecy = { workspace = true }
 
+# Git
+gix = { workspace = true }
+git-url-parse = { workspace = true }
+
 # Webhooks
 ulid = { workspace = true }
 
@@ -30,12 +34,14 @@ ulid = { workspace = true }
 derive_more = { workspace = true }
 derive-with = { workspace = true }
 thiserror = { workspace = true }
+pike = { workspace = true }
 
 [dev-dependencies]
 # # Testing
 assert2 = { workspace = true }
 rand = { workspace = true }
 pretty_assertions = { workspace = true }
+test-log = { workspace = true }
 
 [lints.clippy]
 nursery = { level = "warn", priority = -1 }

crates/config/src/forge_alias.rs

@@ -1,4 +1,4 @@
-crate::newtype!(ForgeAlias: String, Hash, derive_more::Display, Default: "The name of a Forge to connect to");
+crate::newtype!(ForgeAlias: String, Hash, PartialOrd, Ord, derive_more::Display, Default: "The name of a Forge to connect to");
 impl From<&ForgeAlias> for std::path::PathBuf {
     fn from(value: &ForgeAlias) -> Self {
         Self::from(&value.0)

crates/config/src/git_dir.rs

@@ -2,6 +2,7 @@
 use std::path::PathBuf;
 
 use derive_more::Deref;
+use pike::pike;
 
 /// The path to the directory containing the git repository.
 #[derive(
@@ -24,8 +25,16 @@ impl GitDir {
     pub const fn pathbuf(&self) -> &PathBuf {
         &self.pathbuf
     }
-    pub const fn storage_path_type(&self) -> &StoragePathType {
+    pub const fn storage_path_type(&self) -> StoragePathType {
-        &self.storage_path_type
+        self.storage_path_type
+    }
+    pub fn as_fs(&self) -> kxio::fs::FileSystem {
+        pike! {
+            self
+            |> Self::pathbuf
+            |> PathBuf::clone
+            |> kxio::fs::new
+        }
     }
 }
 impl std::fmt::Display for GitDir {
@@ -45,8 +54,13 @@ impl From<&GitDir> for PathBuf {
         value.to_path_buf()
     }
 }
+impl From<&GitDir> for kxio::fs::FileSystem {
+    fn from(gitdir: &GitDir) -> Self {
+        gitdir.as_fs()
+    }
+}
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub enum StoragePathType {
     Internal,
     External,

crates/config/src/lib.rs

@@ -10,6 +10,7 @@ pub mod git_dir;
 mod host_name;
 mod newtype;
 mod registered_webhook;
+mod remote_url;
 mod repo_alias;
 mod repo_branches;
 mod repo_config;
@@ -33,6 +34,7 @@ pub use git_dir::GitDir;
 pub use git_dir::StoragePathType;
 pub use host_name::Hostname;
 pub use registered_webhook::RegisteredWebhook;
+pub use remote_url::RemoteUrl;
 pub use repo_alias::RepoAlias;
 pub use repo_branches::RepoBranches;
 pub use repo_config::RepoConfig;
@@ -43,3 +45,6 @@ pub use user::User;
 pub use webhook::auth::WebhookAuth;
 pub use webhook::forge_notification::ForgeNotification;
 pub use webhook::id::WebhookId;
+
+// re-export
+pub use pike::{pike, pike_opt, pike_res};

crates/config/src/newtype.rs

@@ -28,8 +28,6 @@ macro_rules! newtype {
             derive_more::From,
             PartialEq,
             Eq,
-            PartialOrd,
-            Ord,
             derive_more::AsRef,
             derive_more::Deref,
             $($derive),*

crates/config/src/remote_url.rs

@@ -0,0 +1,16 @@
+crate::newtype!(RemoteUrl: git_url_parse::GitUrl, derive_more::Display: "The URL of a remote repository");
+impl RemoteUrl {
+    pub fn parse(url: impl Into<String>) -> Option<Self> {
+        Some(Self::new(::git_url_parse::GitUrl::parse(&url.into()).ok()?))
+    }
+}
+impl TryFrom<gix::Url> for RemoteUrl {
+    type Error = ();
+
+    fn try_from(url: gix::Url) -> Result<Self, Self::Error> {
+        let pass = url.password().map(|p| p.to_owned());
+        let mut parsed = ::git_url_parse::GitUrl::parse(&url.to_string()).map_err(|_| ())?;
+        parsed.token = pass;
+        Ok(Self(parsed))
+    }
+}

crates/config/src/repo_alias.rs

@@ -2,6 +2,6 @@ use derive_more::Display;
 
 use crate::newtype;
 
-newtype!(RepoAlias: String, Display, Default: r#"The alias of a repo.
+newtype!(RepoAlias: String, Display, Default, PartialOrd, Ord: r#"The alias of a repo.
 
 This is the alias for the repo within `git-next-server.toml`."#);

crates/config/src/tests.rs

@@ -12,6 +12,8 @@ use crate::server::ServerStorage;
 use crate::server::Webhook;
 use crate::webhook::push::Branch;
 
+mod url;
+
 type Result<T> = core::result::Result<T, Box<dyn std::error::Error>>;
 type TestResult = Result<()>;
 

crates/config/src/tests/url.rs

@@ -0,0 +1,25 @@
+use super::*;
+
+#[test_log::test]
+fn url_parse_https_url() -> TestResult {
+    let url = "https://user:pass@git.host/user/repo.git";
+
+    let result = RemoteUrl::parse(url);
+
+    tracing::debug!(?result);
+    assert!(result.is_some());
+
+    Ok(())
+}
+
+#[test_log::test]
+fn url_parse_ssh_url() -> TestResult {
+    let url = "git@git.host:user/repo.git";
+
+    let result = RemoteUrl::parse(url);
+
+    tracing::debug!(?result);
+    assert!(result.is_some());
+
+    Ok(())
+}

crates/git/Cargo.toml

@@ -58,6 +58,7 @@ mockall = { workspace = true }
 assert2 = { workspace = true }
 rand = { workspace = true }
 pretty_assertions = { workspace = true }
+test-log = { workspace = true }
 
 
 [lints.clippy]

crates/git/src/commit.rs

@@ -37,8 +37,8 @@ impl From<config::webhook::Push> for Commit {
     }
 }
 
-newtype!(Sha: String, Display, Hash: "The unique SHA for a git commit.");
+newtype!(Sha: String, Display, Hash,PartialOrd, Ord: "The unique SHA for a git commit.");
-newtype!(Message: String, Hash: "The commit message for a git commit.");
+newtype!(Message: String, Hash, PartialOrd, Ord: "The commit message for a git commit.");
 
 #[derive(Clone, Debug)]
 pub struct Histories {

crates/git/src/repo_details.rs

@@ -1,8 +1,13 @@
+use std::sync::{Arc, RwLock};
+
 use config::{
     BranchName, ForgeAlias, ForgeConfig, ForgeDetails, GitDir, RepoAlias, RepoConfig, RepoPath,
-    ServerRepoConfig,
+    ServerRepoConfig, StoragePathType,
 };
-use git_next_config as config;
+use git_next_config::{self as config, pike, RemoteUrl};
+use secrecy::Secret;
+
+use crate::repository::{OpenRepositoryLike, RealOpenRepository};
 
 use super::{Generation, GitRemote};
 
@@ -55,6 +60,10 @@ impl RepoDetails {
         origin.into()
     }
 
+    pub const fn gitdir(&self) -> &GitDir {
+        &self.gitdir
+    }
+
     pub fn git_remote(&self) -> GitRemote {
         GitRemote::new(self.forge.hostname().clone(), self.repo_path.clone())
     }
@@ -64,4 +73,100 @@ impl RepoDetails {
         self.forge = forge.with_hostname(hostname);
         self
     }
+
+    // url is a secret as it contains auth token
+    pub fn url(&self) -> Secret<String> {
+        let user = self.forge.user();
+        use secrecy::ExposeSecret;
+        let token = self.forge.token().expose_secret();
+        let hostname = self.forge.hostname();
+        let repo_path = &self.repo_path;
+        format!("https://{user}:{token}@{hostname}/{repo_path}.git").into()
+    }
+
+    #[allow(clippy::result_large_err)]
+    pub fn open(&self) -> Result<impl OpenRepositoryLike, crate::validation::remotes::Error> {
+        let gix_repo = pike! {
+            self
+            |> Self::gitdir
+            |> GitDir::pathbuf
+            |> gix::ThreadSafeRepository::open
+        }?;
+        let repo = pike! {
+            gix_repo
+            |> RwLock::new
+            |> Arc::new
+            |> RealOpenRepository::new
+        };
+        Ok(repo)
+    }
+
+    pub fn remote_url(&self) -> Option<git_next_config::RemoteUrl> {
+        use secrecy::ExposeSecret;
+        RemoteUrl::parse(self.url().expose_secret())
+    }
+
+    #[tracing::instrument]
+    pub fn assert_remote_url(&self, found: Option<RemoteUrl>) -> crate::repository::Result<()> {
+        let Some(found) = found else {
+            tracing::debug!("No remote url found to assert");
+            return Ok(());
+        };
+        let Some(expected) = self.remote_url() else {
+            tracing::debug!("No remote url to assert against");
+            return Ok(());
+        };
+        if found != expected {
+            tracing::debug!(?found, ?expected, "urls differ");
+            match self.gitdir.storage_path_type() {
+                StoragePathType::External => {
+                    tracing::debug!("Refusing to update an external repo - user must resolve this");
+                    return Err(crate::repository::Error::MismatchDefaultFetchRemote {
+                        found: Box::new(found),
+                        expected: Box::new(expected),
+                    });
+                }
+                StoragePathType::Internal => {
+                    tracing::debug!(?expected, "Need to update config with new url");
+                    self.write_remote_url(&expected)?;
+                }
+            }
+        }
+        Ok(())
+    }
+
+    #[tracing::instrument]
+    pub fn write_remote_url(&self, url: &RemoteUrl) -> Result<(), kxio::fs::Error> {
+        if self.gitdir.storage_path_type() != StoragePathType::Internal {
+            // return Err(Not an internal repo)
+        }
+        let fs = self.gitdir.as_fs();
+        // load config file
+        let config_filename = &self.gitdir.join("config");
+        let config_file = fs.file_read_to_string(config_filename)?;
+        let mut config_lines = config_file
+            .lines()
+            .map(|l| l.to_owned())
+            .collect::<Vec<_>>();
+        tracing::debug!(?config_lines, "original file");
+        let url_line = format!(r#"   url = "{}""#, url);
+        if config_lines
+            .iter()
+            .any(|line| *line == r#"[remote "origin"]"#)
+        {
+            tracing::debug!("has an 'origin' remote");
+            config_lines
+                .iter_mut()
+                .filter(|line| line.starts_with(r#"   url = "#))
+                .for_each(|line| line.clone_from(&url_line));
+        } else {
+            tracing::debug!("has no 'origin' remote");
+            config_lines.push(r#"[remote "origin"]"#.to_owned());
+            config_lines.push(url_line);
+        }
+        tracing::debug!(?config_lines, "updated file");
+        // write config file back out
+        fs.file_write(config_filename, config_lines.join("\n").as_str())?;
+        Ok(())
+    }
 }

crates/git/src/repository/factory.rs

@@ -1,15 +1,14 @@
 use super::RepoDetails;
 use super::Result;
 pub use crate::repository::open::OpenRepositoryLike;
-use crate::repository::RealOpenRepository;
+use crate::repository::{Direction, RealOpenRepository};
 use derive_more::Deref as _;
-use git_next_config::GitDir;
 use std::sync::{atomic::AtomicBool, Arc, RwLock};
 
 #[mockall::automock]
 pub trait RepositoryFactory: std::fmt::Debug + Sync + Send {
     fn duplicate(&self) -> Box<dyn RepositoryFactory>;
-    fn open(&self, gitdir: &GitDir) -> Result<Box<dyn OpenRepositoryLike>>;
+    fn open(&self, repo_details: &RepoDetails) -> Result<Box<dyn OpenRepositoryLike>>;
     fn git_clone(&self, repo_details: &RepoDetails) -> Result<Box<dyn OpenRepositoryLike>>;
 }
 
@@ -26,9 +25,13 @@ struct RealRepositoryFactory;
 
 #[cfg(not(tarpaulin_include))] // requires network access to either clone new and/or fetch.
 impl RepositoryFactory for RealRepositoryFactory {
-    fn open(&self, gitdir: &GitDir) -> Result<Box<dyn OpenRepositoryLike>> {
+    fn open(&self, repo_details: &RepoDetails) -> Result<Box<dyn OpenRepositoryLike>> {
-        let gix_repo = gix::ThreadSafeRepository::open(gitdir.to_path_buf())?.to_thread_local();
+        let repo = repo_details
-        let repo = RealOpenRepository::new(Arc::new(RwLock::new(gix_repo.into())));
+            .open()
+            .map_err(|e| crate::repository::Error::Open(e.to_string()))?;
+        let found = repo.find_default_remote(Direction::Fetch);
+        repo_details.assert_remote_url(found)?;
+
         Ok(Box::new(repo))
     }
 

crates/git/src/repository/mod.rs

@@ -3,8 +3,7 @@ use super::RepoDetails;
 use crate::repository::test::TestRepository;
 use crate::validation::remotes::validate_default_remotes;
 pub use factory::RepositoryFactory;
-use git_next_config as config;
+use git_next_config::{GitDir, RemoteUrl};
-use git_next_config::GitDir;
 pub use open::otest::OnFetch;
 pub use open::otest::OnPush;
 pub use open::OpenRepository;
@@ -36,14 +35,13 @@ pub const fn test(fs: kxio::fs::FileSystem) -> TestRepository {
 pub fn open(
     repository_factory: &dyn factory::RepositoryFactory,
     repo_details: &RepoDetails,
-    gitdir: config::GitDir,
 ) -> Result<Box<dyn OpenRepositoryLike>> {
-    let open_repository = if !gitdir.exists() {
+    let open_repository = if !repo_details.gitdir.exists() {
         info!("Local copy not found - cloning...");
         repository_factory.git_clone(repo_details)?
     } else {
         info!("Local copy found - opening...");
-        repository_factory.open(&gitdir)?
+        repository_factory.open(repo_details)?
     };
     info!("Validating...");
     validate_default_remotes(&*open_repository, repo_details)
@@ -89,6 +87,9 @@ pub enum Error {
     #[error("invalid git dir: {0}")]
     InvalidGitDir(git_next_config::GitDir),
 
+    #[error("kxiofs: {0}")]
+    KxioFs(#[from] kxio::fs::Error),
+
     #[error("io: {0}")]
     Io(std::io::Error),
 
@@ -112,6 +113,12 @@ pub enum Error {
 
     #[error("fake repository lock")]
     FakeLock,
+
+    #[error("MismatchDefaultFetchRemote(found: {found:?}, expected: {expected:?})")]
+    MismatchDefaultFetchRemote {
+        found: Box<RemoteUrl>,
+        expected: Box<RemoteUrl>,
+    },
 }
 
 mod gix_errors {

crates/git/src/repository/open/mod.rs

@@ -12,7 +12,7 @@ use std::{
 
 use crate as git;
 use git::repository::Direction;
-use git_next_config as config;
+use git_next_config::{self as config, RemoteUrl};
 pub use oreal::RealOpenRepository;
 pub use otest::TestOpenRepository;
 
@@ -53,7 +53,7 @@ pub fn test(
 pub trait OpenRepositoryLike: std::fmt::Debug + Sync {
     fn duplicate(&self) -> Box<dyn OpenRepositoryLike>;
     fn remote_branches(&self) -> git::push::Result<Vec<config::BranchName>>;
-    fn find_default_remote(&self, direction: Direction) -> Option<git::GitRemote>;
+    fn find_default_remote(&self, direction: Direction) -> Option<RemoteUrl>;
     fn fetch(&self) -> Result<(), git::fetch::Error>;
     fn push(
         &self,

crates/git/src/repository/open/oreal.rs

@@ -2,7 +2,7 @@
 use crate::{self as git, repository::OpenRepositoryLike};
 use config::BranchName;
 use derive_more::Constructor;
-use git_next_config as config;
+use git_next_config::{self as config, RemoteUrl};
 
 use gix::bstr::BStr;
 use std::{
@@ -36,17 +36,24 @@ impl super::OpenRepositoryLike for RealOpenRepository {
             })?;
         Ok(refs)
     }
-    fn find_default_remote(&self, direction: git::repository::Direction) -> Option<git::GitRemote> {
+
+    #[tracing::instrument]
+    fn find_default_remote(&self, direction: git::repository::Direction) -> Option<RemoteUrl> {
         let Ok(repository) = self.0.read() else {
             #[cfg(not(tarpaulin_include))] // don't test mutex lock failure
+            tracing::debug!("no repository");
             return None;
         };
         let thread_local = repository.to_thread_local();
         let Some(Ok(remote)) = thread_local.find_default_remote(direction.into()) else {
             #[cfg(not(tarpaulin_include))] // test is on local repo - should always have remotes
+            tracing::debug!("no remote");
             return None;
         };
-        remote.url(direction.into()).map(Into::into)
+        remote
+            .url(direction.into())
+            .cloned()
+            .and_then(|url| RemoteUrl::try_from(url).ok())
     }
 
     #[tracing::instrument(skip_all)]
@@ -216,9 +223,9 @@ fn as_gix_error(branch: BranchName) -> impl FnOnce(String) -> git::commit::log::
     |error| git::commit::log::Error::Gix { branch, error }
 }
 
-impl From<&gix::Url> for git::GitRemote {
+impl From<&RemoteUrl> for git::GitRemote {
-    fn from(url: &gix::Url) -> Self {
+    fn from(url: &RemoteUrl) -> Self {
-        let host = url.host().unwrap_or_default();
+        let host = url.host.clone().unwrap_or_default();
         let path = url.path.to_string();
         let path = path.strip_prefix('/').map_or(path.as_str(), |path| path);
         let path = path.strip_suffix(".git").map_or(path, |path| path);

crates/git/src/repository/open/otest.rs

@@ -1,7 +1,7 @@
 //
 use crate::{self as git, repository::OpenRepositoryLike};
 use derive_more::{Constructor, Deref};
-use git_next_config as config;
+use git_next_config::{self as config, RemoteUrl};
 
 use std::{
     path::Path,
@@ -73,7 +73,7 @@ impl git::repository::OpenRepositoryLike for TestOpenRepository {
         self.real.remote_branches()
     }
 
-    fn find_default_remote(&self, direction: git::repository::Direction) -> Option<git::GitRemote> {
+    fn find_default_remote(&self, direction: git::repository::Direction) -> Option<RemoteUrl> {
         self.real.find_default_remote(direction)
     }
 

crates/git/src/repository/open/tests/fetch.rs

@@ -9,6 +9,7 @@ fn should_fetch_from_repo() {
         cwd.join("../.."),
         config::git_dir::StoragePathType::External,
     );
-    let_assert!(Ok(repo) = crate::repository::factory::real().open(&gitdir));
+    let repo_details = given::repo_details(&given::a_filesystem()).with_gitdir(gitdir);
+    let_assert!(Ok(repo) = crate::repository::factory::real().open(&repo_details));
     let_assert!(Ok(_) = repo.fetch());
 }

crates/git/src/repository/open/tests/find_default_remote.rs

@@ -1,20 +1,13 @@
 use super::*;
 
-#[test]
+#[test_log::test]
 fn should_find_default_push_remote() {
-    // uses the current repo
+    let fs = given::a_filesystem();
-    let_assert!(Ok(cwd) = std::env::current_dir());
+    let gitdir = GitDir::new(fs.base().to_path_buf(), config::StoragePathType::External);
-    let gitdir = config::GitDir::new(
+    let repo_details = given::repo_details(&given::a_filesystem()).with_gitdir(gitdir);
-        cwd.join("../.."),
+    let url = repo_details.url();
-        config::git_dir::StoragePathType::External,
+    given::a_bare_repo_with_url(fs.base(), url.expose_secret(), &fs);
-    ); // from ./crate/git directory to the project root
+    let_assert!(Ok(repo) = git::repository::factory::real().open(&repo_details));
-    let_assert!(Ok(repo) = git::repository::factory::real().open(&gitdir));
+    let remote = repo.find_default_remote(crate::repository::Direction::Push);
-    let_assert!(Some(remote) = repo.find_default_remote(crate::repository::Direction::Push));
+    assert!(remote.is_some());
-    assert_eq!(
-        remote,
-        git::GitRemote::new(
-            config::Hostname::new("git.kemitix.net"),
-            config::RepoPath::new("kemitix/git-next".to_string())
-        )
-    )
 }

crates/git/src/repository/tests/factory.rs

@@ -1,4 +1,4 @@
-use git_next_config::GitDir;
+use git_next_config::{ApiToken, GitDir, StoragePathType};
 
 use super::*;
 
@@ -7,46 +7,134 @@ use crate::tests::given;
 // clone - can't test are this required a remote server (git_clone only works with https origins)
 // open
 // - outside storage path
+// - - auth matches
 #[test_log::test]
-fn open_where_storage_external() -> TestResult {
+fn open_where_storage_external_auth_matches() -> TestResult {
     //given
     let fs = given::a_filesystem();
-    // create a bare repo
+    let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::External);
-    let _x = gix::prepare_clone_bare("https://user:auth@git.kemitix.net/user/repo.git", fs.base())?;
+    let repo_details = given::repo_details(&fs).with_gitdir(gitdir);
-    let gitdir = GitDir::new(
+    use secrecy::ExposeSecret;
-        fs.base().to_path_buf(),
+    let url = repo_details.url();
-        git_next_config::git_dir::Provenance::Internal,
+    let url = url.expose_secret();
-    );
+    given::a_bare_repo_with_url(fs.base(), url, &fs);
     let factory = crate::repository::factory::real();
 
     //when
-    let result = factory.open(&gitdir);
+    let result = factory.open(&repo_details);
 
     //then
     tracing::debug!(?result, "open");
     assert!(result.is_ok());
+    // verify origin in .git/config matches url
+    let config = fs.file_read_to_string(&fs.base().join("config"))?;
+    tracing::debug!(config=?config.lines().collect::<Vec<_>>(), "auth");
+    assert!(config.lines().any(|line| line.contains(url)));
+    Ok(())
+}
+// - outside storage path
+// - - NEW: auth does not match
+// - - - do *NOT* change repo config
+#[test_log::test]
+fn open_where_storage_external_auth_differs_error() -> TestResult {
+    //given
+    let fs = given::a_filesystem();
+    let repo_details = given::repo_details(&fs);
+    use secrecy::ExposeSecret;
+    let original_url = repo_details.url();
+    let original_url = original_url.expose_secret();
+    given::a_bare_repo_with_url(fs.base(), original_url, &fs);
+    let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::External);
+    let factory = crate::repository::factory::real();
+    // create new authentication
+    let api_token = ApiToken::new(given::a_name().into());
+    let forge_details = repo_details.forge.clone().with_token(api_token);
+    let repo_details = repo_details.with_gitdir(gitdir).with_forge(forge_details);
+
+    //when
+    let_assert!(Err(err) = factory.open(&repo_details));
+
+    //then
+    tracing::debug!(?err, "open");
+    assert!(matches!(
+        err,
+        git::repository::Error::MismatchDefaultFetchRemote {
+            found: _,
+            expected: _
+        }
+    ));
+
+    // verify origin in .git/config is unchanged
+    let config = fs.file_read_to_string(&fs.base().join("config"))?;
+    tracing::debug!(config=?config.lines().collect::<Vec<_>>(), "auth");
+    assert!(config.lines().any(|line| line.contains(original_url))); // the original urk
     Ok(())
 }
 // - in storage path
+// - - auth matches
 #[test_log::test]
-fn open_where_storage_internal() -> TestResult {
+fn open_where_storage_internal_auth_matches() -> TestResult {
     //given
     let fs = given::a_filesystem();
-    // create a bare repo
+    let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::Internal);
-    let _x = gix::prepare_clone_bare("https://user:auth@git.kemitix.net/user/repo.git", fs.base())?;
+    let repo_details = given::repo_details(&fs).with_gitdir(gitdir);
-    let gitdir = GitDir::new(
+    use secrecy::ExposeSecret;
-        fs.base().to_path_buf(),
+    let url = repo_details.url();
-        git_next_config::git_dir::Provenance::Internal,
+    let url = url.expose_secret();
-    );
+    // create a bare repg with the auth from the forge_config
+    given::a_bare_repo_with_url(fs.base(), url, &fs);
     let factory = crate::repository::factory::real();
 
     //when
-    let result = factory.open(&gitdir);
+    let result = factory.open(&repo_details);
 
     //then
     tracing::debug!(?result, "open");
     assert!(result.is_ok());
+    // verify origin in .git/config matches url
+    let config = fs.file_read_to_string(&fs.base().join("config"))?;
+    tracing::debug!(config=?config.lines().collect::<Vec<_>>(), "auth");
+    assert!(
+        config.lines().any(|line| line.contains(url)),
+        "Mismatched URL should not be updated"
+    );
     Ok(())
 }
-// - - auth matches
+// - in storage path
 // - - NEW: auth does not match
+// - - - CHANGE repo config to match updated auth
+#[test_log::test]
+fn open_where_storage_internal_auth_differs_update_config() -> TestResult {
+    //given
+    let fs = given::a_filesystem();
+    let repo_details = given::repo_details(&fs);
+    use secrecy::ExposeSecret;
+    let original_url = repo_details.url();
+    let original_url = original_url.expose_secret();
+    given::a_bare_repo_with_url(fs.base(), original_url, &fs);
+    let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::Internal);
+    let factory = crate::repository::factory::real();
+    // create new authentication
+    let api_token = ApiToken::new(given::a_name().into());
+    let forge_details = repo_details.forge.clone().with_token(api_token);
+    let repo_details = repo_details.with_gitdir(gitdir).with_forge(forge_details);
+    let updated_url = repo_details.url();
+    let updated_url = updated_url.expose_secret().to_lowercase();
+
+    //when
+    let result = factory.open(&repo_details);
+
+    //then
+    assert!(result.is_ok());
+
+    // verify origin in .git/config is unchanged
+    let config = fs.file_read_to_string(&fs.base().join("config"))?;
+    tracing::debug!(config=?config.lines().collect::<Vec<_>>(), "auth");
+    assert!(
+        config
+            .lines()
+            .any(|line| line.to_lowercase().contains(&updated_url)),
+        "Updated URL should be written to config file"
+    );
+    Ok(())
+}

crates/git/src/repository/tests/mod.rs

@@ -1,3 +1,7 @@
 use crate as git;
+use assert2::let_assert;
 
+mod factory;
 mod validate;
+
+type TestResult = Result<(), Box<dyn std::error::Error>>;

crates/git/src/repository/tests/validate.rs

@@ -10,7 +10,7 @@ fn should_ok_a_valid_repo() {
     let mut open_repository = git::repository::open::mock();
     open_repository
         .expect_find_default_remote()
-        .returning(move |_direction| Some(repo_details_mock.git_remote()));
+        .returning(move |_direction| repo_details_mock.remote_url());
 
     let result = validate_default_remotes(&*open_repository, &repo_details);
     println!("{result:?}");
@@ -28,7 +28,7 @@ fn should_fail_where_no_default_push_remote() {
         .expect_find_default_remote()
         .returning(move |direction| match direction {
             Direction::Push => None,
-            Direction::Fetch => Some(repo_details_mock.git_remote()),
+            Direction::Fetch => repo_details_mock.remote_url(),
         });
 
     let result = validate_default_remotes(&*open_repository, &repo_details);
@@ -46,7 +46,7 @@ fn should_fail_where_no_default_fetch_remote() {
     open_repository
         .expect_find_default_remote()
         .returning(move |direction| match direction {
-            Direction::Push => Some(repo_details_mock.git_remote()),
+            Direction::Push => repo_details_mock.remote_url(),
             Direction::Fetch => None,
         });
 
@@ -65,8 +65,8 @@ fn should_fail_where_invalid_default_push_remote() {
     open_repository
         .expect_find_default_remote()
         .returning(move |direction| match direction {
-            Direction::Push => Some(given::a_git_remote()),
+            Direction::Push => Some(given::a_remote_url()),
-            Direction::Fetch => Some(repo_details_mock.git_remote()),
+            Direction::Fetch => repo_details_mock.remote_url(),
         });
 
     let result = validate_default_remotes(&*open_repository, &repo_details);
@@ -84,8 +84,8 @@ fn should_fail_where_invalid_default_fetch_remote() {
     open_repository
         .expect_find_default_remote()
         .returning(move |direction| match direction {
-            Direction::Push => Some(repo_details_mock.git_remote()),
+            Direction::Push => repo_details_mock.remote_url(),
-            Direction::Fetch => Some(given::a_git_remote()),
+            Direction::Fetch => Some(given::a_remote_url()),
         });
 
     let result = validate_default_remotes(&*open_repository, &repo_details);

crates/git/src/tests.rs

@@ -214,7 +214,7 @@ mod repo_details {
     }
 }
 pub mod given {
-    use std::path::PathBuf;
+    use std::path::{Path, PathBuf};
 
     use crate::{self as git, repository::open::MockOpenRepositoryLike};
     use config::{
@@ -270,9 +270,9 @@ pub mod given {
     pub fn a_forge_config() -> ForgeConfig {
         ForgeConfig::new(
             ForgeType::MockForge,
-            a_name(),
+            format!("hostname-{}", a_name()),
-            a_name(),
+            format!("user-{}", a_name()),
-            a_name(),
+            format!("token-{}", a_name()),
             Default::default(), // no repos
         )
     }
@@ -369,6 +369,32 @@ pub mod given {
             config::RepoPath::new(given::a_name()),
         )
     }
+
+    #[allow(clippy::unwrap_used)]
+    pub fn a_bare_repo_with_url(path: &Path, url: &str, fs: &kxio::fs::FileSystem) {
+        // create a basic bare repo
+        let repo = gix::prepare_clone_bare(url, fs.base()).unwrap();
+        repo.persist();
+        // load config file
+        let config_file = fs.file_read_to_string(&path.join("config")).unwrap();
+        // add use are origin url
+        let mut config_lines = config_file.lines().collect::<Vec<_>>();
+        config_lines.push(r#"[remote "origin"]"#);
+        let url_line = format!(r#"   url = "{}""#, url);
+        tracing::info!(?url, %url_line, "writing");
+        config_lines.push(&url_line);
+        // write config file back out
+        fs.file_write(&path.join("config"), config_lines.join("\n").as_str())
+            .unwrap();
+    }
+
+    #[allow(clippy::unwrap_used)]
+    pub fn a_remote_url() -> git_next_config::RemoteUrl {
+        let hostname = given::a_hostname();
+        let owner = given::a_name();
+        let repo = given::a_name();
+        git_next_config::RemoteUrl::parse(format!("git@{hostname}:{owner}/{repo}.git")).unwrap()
+    }
 }
 pub mod then {
     use std::path::{Path, PathBuf};

crates/git/src/validation/remotes.rs

@@ -1,3 +1,4 @@
+use git_next_config::RemoteUrl;
 use tracing::info;
 
 use crate as git;
@@ -13,18 +14,22 @@ pub fn validate_default_remotes(
     let fetch_remote = open_repository
         .find_default_remote(git::repository::Direction::Fetch)
         .ok_or_else(|| Error::NoDefaultFetchRemote)?;
-    let git_remote = repo_details.git_remote();
+    let Some(remote_url) = repo_details.remote_url() else {
-    info!(config = %git_remote, push = %push_remote, fetch = %fetch_remote, "Check remotes match");
+        return Err(git::validation::remotes::Error::UnableToOpenRepo(
-    if git_remote != push_remote {
+            "Unable to build forge url".to_string(),
+        ));
+    };
+    info!(config = %remote_url, push = %push_remote, fetch = %fetch_remote, "Check remotes match");
+    if remote_url != push_remote {
         return Err(Error::MismatchDefaultPushRemote {
             found: push_remote,
-            expected: git_remote,
+            expected: remote_url,
         });
     }
-    if git_remote != fetch_remote {
+    if remote_url != fetch_remote {
         return Err(Error::MismatchDefaultFetchRemote {
             found: fetch_remote,
-            expected: git_remote,
+            expected: remote_url,
         });
     }
     Ok(())
@@ -53,13 +58,16 @@ pub enum Error {
 
     #[error("MismatchDefaultPushRemote(found: {found}, expected: {expected})")]
     MismatchDefaultPushRemote {
-        found: git::GitRemote,
+        found: RemoteUrl,
-        expected: git::GitRemote,
+        expected: RemoteUrl,
     },
 
     #[error("MismatchDefaultFetchRemote(found: {found}, expected: {expected})")]
     MismatchDefaultFetchRemote {
-        found: git::GitRemote,
+        found: RemoteUrl,
-        expected: git::GitRemote,
+        expected: RemoteUrl,
     },
+
+    #[error("Unable to open repo")]
+    GixOpen(#[from] gix::open::Error),
 }

crates/git/src/validation/tests.rs

@@ -20,6 +20,7 @@ mod repos {
     fn where_repo_has_no_push_remote() {
         let_assert!(Ok(fs) = kxio::fs::temp(), "temp fs");
         let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::Internal);
+        let repo_details = given::repo_details(&fs).with_gitdir(gitdir);
         let mut mock_open_repository = git::repository::open::mock();
         mock_open_repository
             .expect_find_default_remote()
@@ -29,9 +30,8 @@ mod repos {
         repository_factory
             .expect_open()
             .return_once(move |_| Ok(mock_open_repository));
-        let repo_details = given::repo_details(&fs);
         let_assert!(
-            Ok(open_repository) = repository_factory.open(&gitdir),
+            Ok(open_repository) = repository_factory.open(&repo_details),
             "open repo"
         );
 
@@ -62,6 +62,7 @@ mod positions {
         fn where_fetch_fails_should_error() {
             let fs = given::a_filesystem();
             let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::Internal);
+            let repo_details = given::repo_details(&fs).with_gitdir(gitdir);
             let mut mock_open_repository = git::repository::open::mock();
             mock_open_repository
                 .expect_fetch()
@@ -71,10 +72,9 @@ mod positions {
                 .expect_open()
                 .return_once(move |_| Ok(mock_open_repository));
             let_assert!(
-                Ok(repository) = repository_factory.open(&gitdir),
+                Ok(repository) = repository_factory.open(&repo_details),
                 "open repo"
             );
-            let repo_details = given::repo_details(&fs); //.with_gitdir(gitdir);
             let repo_config = given::a_repo_config();
 
             let result = validate_positions(&*repository, &repo_details, repo_config);
@@ -91,6 +91,7 @@ mod positions {
         fn where_main_branch_is_missing_or_commit_log_is_empty_should_error() {
             let fs = given::a_filesystem();
             let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::Internal);
+            let repo_details = given::repo_details(&fs).with_gitdir(gitdir);
             let repo_config = given::a_repo_config();
             let main_branch = repo_config.branches().main();
             let mut mock_open_repository = git::repository::open::mock();
@@ -112,10 +113,9 @@ mod positions {
                 .expect_open()
                 .return_once(move |_| Ok(mock_open_repository));
             let_assert!(
-                Ok(open_repository) = repository_factory.open(&gitdir),
+                Ok(open_repository) = repository_factory.open(&repo_details),
                 "open repo"
             );
-            let repo_details = given::repo_details(&fs);
 
             let result = validate_positions(&*open_repository, &repo_details, repo_config);
             println!("{result:?}");
@@ -130,6 +130,7 @@ mod positions {
         fn where_next_branch_is_missing_or_commit_log_is_empty_should_error() {
             let fs = given::a_filesystem();
             let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::Internal);
+            let repo_details = given::repo_details(&fs).with_gitdir(gitdir);
             let repo_config = given::a_repo_config();
             let next_branch = repo_config.branches().next();
             let mut mock_open_repository = git::repository::open::mock();
@@ -151,10 +152,9 @@ mod positions {
                 .expect_open()
                 .return_once(move |_| Ok(mock_open_repository));
             let_assert!(
-                Ok(open_repository) = repository_factory.open(&gitdir),
+                Ok(open_repository) = repository_factory.open(&repo_details),
                 "open repo"
             );
-            let repo_details = given::repo_details(&fs);
 
             let result = validate_positions(&*open_repository, &repo_details, repo_config);
             println!("{result:?}");
@@ -169,6 +169,7 @@ mod positions {
         fn where_dev_branch_is_missing_or_commit_log_is_empty_should_error() {
             let fs = given::a_filesystem();
             let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::Internal);
+            let repo_details = given::repo_details(&fs).with_gitdir(gitdir);
             let repo_config = given::a_repo_config();
             let dev_branch = repo_config.branches().dev();
             let mut mock_open_repository = git::repository::open::mock();
@@ -190,10 +191,9 @@ mod positions {
                 .expect_open()
                 .return_once(move |_| Ok(mock_open_repository));
             let_assert!(
-                Ok(open_repository) = repository_factory.open(&gitdir),
+                Ok(open_repository) = repository_factory.open(&repo_details),
                 "open repo"
             );
-            let repo_details = given::repo_details(&fs);
 
             let result = validate_positions(&*open_repository, &repo_details, repo_config);
             println!("{result:?}");

crates/repo-actor/src/handlers/clone_repo.rs

@@ -14,8 +14,7 @@ impl Handler<actor::messages::CloneRepo> for actor::RepoActor {
     ) -> Self::Result {
         actor::logger(&self.log, "Handler: CloneRepo: start");
         tracing::debug!("Handler: CloneRepo: start");
-        let gitdir = self.repo_details.gitdir.clone();
+        match git::repository::open(&*self.repository_factory, &self.repo_details) {
-        match git::repository::open(&*self.repository_factory, &self.repo_details, gitdir) {
             Ok(repository) => {
                 actor::logger(&self.log, "open okay");
                 tracing::debug!("open okay");

crates/repo-actor/src/messages.rs

@@ -38,7 +38,7 @@ impl From<config::RegisteredWebhook> for WebhookRegistered {
     }
 }
 
-newtype!(MessageToken: u32, Copy, Default, Display: r#"An incremental token used to identify the current set of messages.
+newtype!(MessageToken: u32, Copy, Default, Display, PartialOrd, Ord: r#"An incremental token used to identify the current set of messages.
 
 Primarily used by [ValidateRepo] to reduce duplicate messages. The token is incremented when a new Webhook message is
 received, marking that message the latest, and causing any previous messages still being processed to be dropped when

crates/repo-actor/src/tests/given.rs

@@ -1,4 +1,5 @@
 use config::git_dir::StoragePathType;
+use git_next_config::RemoteUrl;
 
 //
 use super::*;
@@ -10,15 +11,15 @@ pub fn has_all_valid_remote_defaults(
     has_remote_defaults(
         open_repository,
         HashMap::from([
-            (Direction::Push, Some(repo_details.git_remote())),
+            (Direction::Push, repo_details.remote_url()),
-            (Direction::Fetch, Some(repo_details.git_remote())),
+            (Direction::Fetch, repo_details.remote_url()),
         ]),
     );
 }
 
 pub fn has_remote_defaults(
     open_repository: &mut MockOpenRepositoryLike,
-    remotes: HashMap<Direction, Option<GitRemote>>,
+    remotes: HashMap<Direction, Option<RemoteUrl>>,
 ) {
     remotes.into_iter().for_each(|(direction, remote)| {
         open_repository

crates/repo-actor/src/tests/handlers/clone_repo.rs

@@ -42,13 +42,8 @@ async fn should_clone() -> TestResult {
 async fn should_open() -> TestResult {
     //given
     let fs = given::a_filesystem();
-    let (mut open_repository, /* mut */ repo_details) = given::an_open_repository(&fs);
+    let (mut open_repository, repo_details) = given::an_open_repository(&fs);
-    // #[allow(clippy::unwrap_used)]
-    // let repo_config = repo_details.repo_config.take().unwrap();
-
     given::has_all_valid_remote_defaults(&mut open_repository, &repo_details);
-    // handles_validate_repo_message(&mut open_repository, repo_config.branches());
-
     // factory opens a repository
     let mut repository_factory = MockRepositoryFactory::new();
     let opened = Arc::new(RwLock::new(vec![]));
@@ -139,7 +134,7 @@ async fn opened_repo_with_no_default_push_should_not_proceed() -> TestResult {
         &mut open_repository,
         HashMap::from([
             (Direction::Push, None),
-            (Direction::Fetch, Some(repo_details.git_remote())),
+            (Direction::Fetch, repo_details.remote_url()),
         ]),
     );
 
@@ -167,7 +162,7 @@ async fn opened_repo_with_no_default_fetch_should_not_proceed() -> TestResult {
     given::has_remote_defaults(
         &mut open_repository,
         HashMap::from([
-            (Direction::Push, Some(repo_details.git_remote())),
+            (Direction::Push, repo_details.remote_url()),
             (Direction::Fetch, None),
         ]),
     );

crates/repo-actor/src/tests/handlers/validate_repo.rs

@@ -354,6 +354,7 @@ async fn should_reject_message_with_expired_token() -> TestResult {
 }
 
 #[test_log::test(actix::test)]
+// NOTE: failed then passed on retry: count = 1
 async fn should_send_validate_repo_when_retryable_error() -> TestResult {
     //given
     let fs = given::a_filesystem();

crates/repo-actor/src/tests/mod.rs

@@ -14,7 +14,7 @@ use config::{
 };
 use git::{
     repository::{factory::MockRepositoryFactory, open::MockOpenRepositoryLike, Direction},
-    Generation, GitRemote, MockForgeLike, RepoDetails,
+    Generation, MockForgeLike, RepoDetails,
 };
 use git_next_actor_macros::message;
 use git_next_config as config;