Skip to content

fix 1912 #1964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions gix-config/src/file/access/read_only.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = SectionId> + '_ {
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.
///
Expand Down
14 changes: 11 additions & 3 deletions gix-config/src/file/mutable/section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ use std::{
ops::{Deref, Range},
};

use bstr::{BStr, BString, ByteSlice, ByteVec};
use smallvec::SmallVec;

use crate::{
file::{
self,
Expand All @@ -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)]
Expand Down Expand Up @@ -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<Cow<'event, BStr>> {
let key = ValueName::from_str_unchecked(value_name);
Expand Down
File renamed without changes.
File renamed without changes.
94 changes: 71 additions & 23 deletions gix/src/open/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand All @@ -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<BString> = 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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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 })
}
}
Loading