From c33d154cb5ff03dfd4ea0950404ec45b70c27e79 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 21 Apr 2025 10:44:33 +0200 Subject: [PATCH 1/3] refactor tests --- gix-sec/tests/{identity/mod.rs => sec/identity.rs} | 0 gix-sec/tests/{sec.rs => sec/main.rs} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename gix-sec/tests/{identity/mod.rs => sec/identity.rs} (100%) rename gix-sec/tests/{sec.rs => sec/main.rs} (100%) diff --git a/gix-sec/tests/identity/mod.rs b/gix-sec/tests/sec/identity.rs similarity index 100% rename from gix-sec/tests/identity/mod.rs rename to gix-sec/tests/sec/identity.rs diff --git a/gix-sec/tests/sec.rs b/gix-sec/tests/sec/main.rs similarity index 100% rename from gix-sec/tests/sec.rs rename to gix-sec/tests/sec/main.rs From aa91f9ae4c589efb5b047c1f0d9be7fe49f55ab0 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 21 Apr 2025 14:19:15 +0200 Subject: [PATCH 2/3] feat: add `File::section_ids()` iterator and `file::SectionMut::set_trust()` #(1912) That way it's easier to make changes to sections in a mutable instance of Git configuration. --- gix-config/src/file/access/read_only.rs | 5 +++++ gix-config/src/file/mutable/section.rs | 14 +++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/gix-config/src/file/access/read_only.rs b/gix-config/src/file/access/read_only.rs index e433bd4d0ae..308b26766d2 100644 --- a/gix-config/src/file/access/read_only.rs +++ b/gix-config/src/file/access/read_only.rs @@ -410,6 +410,11 @@ impl<'event> File<'event> { self.section_order.iter().map(move |id| (&self.sections[id], *id)) } + /// Return an iterator over all section ids, in order of occurrence in the file itself. + pub fn section_ids(&mut self) -> impl Iterator + '_ { + self.section_order.iter().copied() + } + /// Return an iterator over all sections along with non-section events that are placed right after them, /// in order of occurrence in the file itself. /// diff --git a/gix-config/src/file/mutable/section.rs b/gix-config/src/file/mutable/section.rs index 447c8648d67..d41f2f6e8f5 100644 --- a/gix-config/src/file/mutable/section.rs +++ b/gix-config/src/file/mutable/section.rs @@ -3,9 +3,6 @@ use std::{ ops::{Deref, Range}, }; -use bstr::{BStr, BString, ByteSlice, ByteVec}; -use smallvec::SmallVec; - use crate::{ file::{ self, @@ -16,6 +13,9 @@ use crate::{ parse::{section::ValueName, Event}, value::{normalize, normalize_bstr, normalize_bstring}, }; +use bstr::{BStr, BString, ByteSlice, ByteVec}; +use gix_sec::Trust; +use smallvec::SmallVec; /// A opaque type that represents a mutable reference to a section. #[derive(PartialEq, Eq, Hash, PartialOrd, Ord, Debug)] @@ -142,6 +142,14 @@ impl<'event> SectionMut<'_, 'event> { } } + /// Set the trust level in the meta-data of this section to `trust`. + pub fn set_trust(&mut self, trust: Trust) -> &mut Self { + let mut meta = (*self.section.meta).clone(); + meta.trust = trust; + self.section.meta = meta.into(); + self + } + /// Removes the latest value by key and returns it, if it exists. pub fn remove(&mut self, value_name: &str) -> Option> { let key = ValueName::from_str_unchecked(value_name); From 24d235d7a6bdb01c5c1cff79b5be14f1d5afa11a Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 21 Apr 2025 14:21:08 +0200 Subject: [PATCH 3/3] fix: `safe.directory` now applies to configuration as well (#1912) This means that repo-local configuration that is considered safe, ideally with `safe.directory=safe/dir/*` notation, will be usable for sensitive operations. --- gix/src/open/repository.rs | 94 ++++++++++++++++++++++++++++---------- 1 file changed, 71 insertions(+), 23 deletions(-) diff --git a/gix/src/open/repository.rs b/gix/src/open/repository.rs index bb3e15e26ee..55a2db1a111 100644 --- a/gix/src/open/repository.rs +++ b/gix/src/open/repository.rs @@ -233,7 +233,7 @@ impl ThreadSafeRepository { let home = gix_path::env::home_dir().and_then(|home| env.home.check_opt(home)); let mut filter_config_section = filter_config_section.unwrap_or(config::section::is_trusted); - let config = config::Cache::from_stage_one( + let mut config = config::Cache::from_stage_one( repo_config, common_dir_ref, head.as_ref().and_then(|head| head.target.try_name()), @@ -248,14 +248,54 @@ impl ThreadSafeRepository { cli_config_overrides, )?; - if bail_if_untrusted && git_dir_trust != gix_sec::Trust::Full { - check_safe_directories( - &git_dir, - git_install_dir.as_deref(), - current_dir, - home.as_deref(), - &config, - )?; + // TODO: Testing - it's hard to get non-ownership reliably and without root. + // For now tested manually with https://github.com/GitoxideLabs/gitoxide/issues/1912 + if git_dir_trust != gix_sec::Trust::Full { + let safe_dirs: Vec = config + .resolved + .strings_filter(Safe::DIRECTORY, &mut Safe::directory_filter) + .unwrap_or_default() + .into_iter() + .map(Cow::into_owned) + .collect(); + if bail_if_untrusted { + check_safe_directories( + &git_dir, + git_install_dir.as_deref(), + current_dir, + home.as_deref(), + &safe_dirs, + )?; + } + let Ok(mut resolved) = gix_features::threading::OwnShared::try_unwrap(config.resolved) else { + unreachable!("Shared ownership was just established, with one reference") + }; + let section_ids: Vec<_> = resolved.section_ids().collect(); + for id in section_ids { + let Some(mut section) = resolved.section_mut_by_id(id) else { + continue; + }; + let section_trusted_by_default = Safe::directory_filter(section.meta()); + if section_trusted_by_default || section.meta().trust == gix_sec::Trust::Full { + continue; + } + let Some(meta_path) = section.meta().path.as_deref() else { + continue; + }; + let config_file_is_safe = check_safe_directories( + meta_path, + git_install_dir.as_deref(), + current_dir, + home.as_deref(), + &safe_dirs, + ) + .is_ok(); + + if config_file_is_safe { + section.set_trust(gix_sec::Trust::Full); + } + } + config.resolved = resolved.into(); } // core.worktree might be used to overwrite the worktree directory @@ -423,23 +463,20 @@ fn replacement_objects_refs_prefix( } fn check_safe_directories( - git_dir: &std::path::Path, + path_to_test: &std::path::Path, git_install_dir: Option<&std::path::Path>, current_dir: &std::path::Path, home: Option<&std::path::Path>, - config: &config::Cache, + safe_dirs: &[BString], ) -> Result<(), Error> { let mut is_safe = false; - let git_dir = match gix_path::realpath_opts(git_dir, current_dir, gix_path::realpath::MAX_SYMLINKS) { + let path_to_test = match gix_path::realpath_opts(path_to_test, current_dir, gix_path::realpath::MAX_SYMLINKS) { Ok(p) => p, - Err(_) => git_dir.to_owned(), + Err(_) => path_to_test.to_owned(), }; - for safe_dir in config - .resolved - .strings_filter(Safe::DIRECTORY, &mut Safe::directory_filter) - .unwrap_or_default() - { - if safe_dir.as_ref() == "*" { + for safe_dir in safe_dirs { + let safe_dir = safe_dir.as_bstr(); + if safe_dir == "*" { is_safe = true; continue; } @@ -448,21 +485,32 @@ fn check_safe_directories( continue; } if !is_safe { - let safe_dir = match gix_config::Path::from(std::borrow::Cow::Borrowed(safe_dir.as_ref())) + let safe_dir = match gix_config::Path::from(Cow::Borrowed(safe_dir)) .interpolate(interpolate_context(git_install_dir, home)) { Ok(path) => path, Err(_) => gix_path::from_bstr(safe_dir), }; - if safe_dir == git_dir { - is_safe = true; + if !safe_dir.is_absolute() { + gix_trace::warn!( + "safe.directory '{safe_dir}' not absolute", + safe_dir = safe_dir.display() + ); continue; } + if safe_dir.ends_with("*") { + let safe_dir = safe_dir.parent().expect("* is last component"); + if path_to_test.strip_prefix(safe_dir).is_ok() { + is_safe = true; + } + } else if safe_dir == path_to_test { + is_safe = true; + } } } if is_safe { Ok(()) } else { - Err(Error::UnsafeGitDir { path: git_dir }) + Err(Error::UnsafeGitDir { path: path_to_test }) } }