Skip to content

Commit 24d235d

Browse files
committed
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.
1 parent aa91f9a commit 24d235d

File tree

1 file changed

+71
-23
lines changed

1 file changed

+71
-23
lines changed

gix/src/open/repository.rs

Lines changed: 71 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ impl ThreadSafeRepository {
233233
let home = gix_path::env::home_dir().and_then(|home| env.home.check_opt(home));
234234

235235
let mut filter_config_section = filter_config_section.unwrap_or(config::section::is_trusted);
236-
let config = config::Cache::from_stage_one(
236+
let mut config = config::Cache::from_stage_one(
237237
repo_config,
238238
common_dir_ref,
239239
head.as_ref().and_then(|head| head.target.try_name()),
@@ -248,14 +248,54 @@ impl ThreadSafeRepository {
248248
cli_config_overrides,
249249
)?;
250250

251-
if bail_if_untrusted && git_dir_trust != gix_sec::Trust::Full {
252-
check_safe_directories(
253-
&git_dir,
254-
git_install_dir.as_deref(),
255-
current_dir,
256-
home.as_deref(),
257-
&config,
258-
)?;
251+
// TODO: Testing - it's hard to get non-ownership reliably and without root.
252+
// For now tested manually with https://github.com/GitoxideLabs/gitoxide/issues/1912
253+
if git_dir_trust != gix_sec::Trust::Full {
254+
let safe_dirs: Vec<BString> = config
255+
.resolved
256+
.strings_filter(Safe::DIRECTORY, &mut Safe::directory_filter)
257+
.unwrap_or_default()
258+
.into_iter()
259+
.map(Cow::into_owned)
260+
.collect();
261+
if bail_if_untrusted {
262+
check_safe_directories(
263+
&git_dir,
264+
git_install_dir.as_deref(),
265+
current_dir,
266+
home.as_deref(),
267+
&safe_dirs,
268+
)?;
269+
}
270+
let Ok(mut resolved) = gix_features::threading::OwnShared::try_unwrap(config.resolved) else {
271+
unreachable!("Shared ownership was just established, with one reference")
272+
};
273+
let section_ids: Vec<_> = resolved.section_ids().collect();
274+
for id in section_ids {
275+
let Some(mut section) = resolved.section_mut_by_id(id) else {
276+
continue;
277+
};
278+
let section_trusted_by_default = Safe::directory_filter(section.meta());
279+
if section_trusted_by_default || section.meta().trust == gix_sec::Trust::Full {
280+
continue;
281+
}
282+
let Some(meta_path) = section.meta().path.as_deref() else {
283+
continue;
284+
};
285+
let config_file_is_safe = check_safe_directories(
286+
meta_path,
287+
git_install_dir.as_deref(),
288+
current_dir,
289+
home.as_deref(),
290+
&safe_dirs,
291+
)
292+
.is_ok();
293+
294+
if config_file_is_safe {
295+
section.set_trust(gix_sec::Trust::Full);
296+
}
297+
}
298+
config.resolved = resolved.into();
259299
}
260300

261301
// core.worktree might be used to overwrite the worktree directory
@@ -423,23 +463,20 @@ fn replacement_objects_refs_prefix(
423463
}
424464

425465
fn check_safe_directories(
426-
git_dir: &std::path::Path,
466+
path_to_test: &std::path::Path,
427467
git_install_dir: Option<&std::path::Path>,
428468
current_dir: &std::path::Path,
429469
home: Option<&std::path::Path>,
430-
config: &config::Cache,
470+
safe_dirs: &[BString],
431471
) -> Result<(), Error> {
432472
let mut is_safe = false;
433-
let git_dir = match gix_path::realpath_opts(git_dir, current_dir, gix_path::realpath::MAX_SYMLINKS) {
473+
let path_to_test = match gix_path::realpath_opts(path_to_test, current_dir, gix_path::realpath::MAX_SYMLINKS) {
434474
Ok(p) => p,
435-
Err(_) => git_dir.to_owned(),
475+
Err(_) => path_to_test.to_owned(),
436476
};
437-
for safe_dir in config
438-
.resolved
439-
.strings_filter(Safe::DIRECTORY, &mut Safe::directory_filter)
440-
.unwrap_or_default()
441-
{
442-
if safe_dir.as_ref() == "*" {
477+
for safe_dir in safe_dirs {
478+
let safe_dir = safe_dir.as_bstr();
479+
if safe_dir == "*" {
443480
is_safe = true;
444481
continue;
445482
}
@@ -448,21 +485,32 @@ fn check_safe_directories(
448485
continue;
449486
}
450487
if !is_safe {
451-
let safe_dir = match gix_config::Path::from(std::borrow::Cow::Borrowed(safe_dir.as_ref()))
488+
let safe_dir = match gix_config::Path::from(Cow::Borrowed(safe_dir))
452489
.interpolate(interpolate_context(git_install_dir, home))
453490
{
454491
Ok(path) => path,
455492
Err(_) => gix_path::from_bstr(safe_dir),
456493
};
457-
if safe_dir == git_dir {
458-
is_safe = true;
494+
if !safe_dir.is_absolute() {
495+
gix_trace::warn!(
496+
"safe.directory '{safe_dir}' not absolute",
497+
safe_dir = safe_dir.display()
498+
);
459499
continue;
460500
}
501+
if safe_dir.ends_with("*") {
502+
let safe_dir = safe_dir.parent().expect("* is last component");
503+
if path_to_test.strip_prefix(safe_dir).is_ok() {
504+
is_safe = true;
505+
}
506+
} else if safe_dir == path_to_test {
507+
is_safe = true;
508+
}
461509
}
462510
}
463511
if is_safe {
464512
Ok(())
465513
} else {
466-
Err(Error::UnsafeGitDir { path: git_dir })
514+
Err(Error::UnsafeGitDir { path: path_to_test })
467515
}
468516
}

0 commit comments

Comments
 (0)