Skip to content

Commit ca16517

Browse files
committed
fix: correctly handle safe.directory for worktrees (#1912)
1 parent 359914c commit ca16517

File tree

2 files changed

+83
-53
lines changed

2 files changed

+83
-53
lines changed

gix/src/open/repository.rs

+82-53
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ use crate::{
1313
use gix_features::threading::OwnShared;
1414
use gix_object::bstr::ByteSlice;
1515
use gix_path::RelativePath;
16+
use std::collections::btree_map::Entry;
17+
use std::collections::BTreeMap;
1618
use std::ffi::OsStr;
1719
use std::{borrow::Cow, path::PathBuf};
1820

@@ -157,7 +159,7 @@ impl ThreadSafeRepository {
157159
) -> Result<Self, Error> {
158160
let _span = gix_trace::detail!("open_from_paths()");
159161
let Options {
160-
git_dir_trust,
162+
ref mut git_dir_trust,
161163
object_store_slots,
162164
filter_config_section,
163165
lossy_config,
@@ -174,15 +176,15 @@ impl ThreadSafeRepository {
174176
ref cli_config_overrides,
175177
ref mut current_dir,
176178
} = options;
177-
let git_dir_trust = git_dir_trust.expect("trust must be determined by now");
179+
let git_dir_trust = git_dir_trust.as_mut().expect("trust must be determined by now");
178180

179181
let mut common_dir = gix_discover::path::from_plain_file(git_dir.join("commondir").as_ref())
180182
.transpose()?
181183
.map(|cd| git_dir.join(cd));
182184
let repo_config = config::cache::StageOne::new(
183185
common_dir.as_deref().unwrap_or(&git_dir),
184186
git_dir.as_ref(),
185-
git_dir_trust,
187+
*git_dir_trust,
186188
lossy_config,
187189
lenient_config,
188190
)?;
@@ -248,56 +250,6 @@ impl ThreadSafeRepository {
248250
cli_config_overrides,
249251
)?;
250252

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();
299-
}
300-
301253
// core.worktree might be used to overwrite the worktree directory
302254
if !config.is_bare {
303255
let mut key_source = None;
@@ -384,6 +336,83 @@ impl ThreadSafeRepository {
384336
}
385337
}
386338

339+
// TODO: Testing - it's hard to get non-ownership reliably and without root.
340+
// For now tested manually with https://github.com/GitoxideLabs/gitoxide/issues/1912
341+
if *git_dir_trust != gix_sec::Trust::Full
342+
|| worktree_dir
343+
.as_deref()
344+
.is_some_and(|wd| !gix_sec::identity::is_path_owned_by_current_user(wd).unwrap_or(false))
345+
{
346+
let safe_dirs: Vec<BString> = config
347+
.resolved
348+
.strings_filter(Safe::DIRECTORY, &mut Safe::directory_filter)
349+
.unwrap_or_default()
350+
.into_iter()
351+
.map(Cow::into_owned)
352+
.collect();
353+
let test_dir = worktree_dir.as_deref().unwrap_or(git_dir.as_path());
354+
let res = check_safe_directories(
355+
test_dir,
356+
git_install_dir.as_deref(),
357+
current_dir,
358+
home.as_deref(),
359+
&safe_dirs,
360+
);
361+
if res.is_ok() {
362+
*git_dir_trust = gix_sec::Trust::Full;
363+
} else if bail_if_untrusted {
364+
res?;
365+
} else {
366+
// This is how the worktree-trust can reduce the git-dir trust.
367+
*git_dir_trust = gix_sec::Trust::Reduced;
368+
}
369+
370+
let Ok(mut resolved) = gix_features::threading::OwnShared::try_unwrap(config.resolved) else {
371+
unreachable!("Shared ownership was just established, with one reference")
372+
};
373+
let section_ids: Vec<_> = resolved.section_ids().collect();
374+
let mut is_valid_by_path = BTreeMap::new();
375+
for id in section_ids {
376+
let Some(mut section) = resolved.section_mut_by_id(id) else {
377+
continue;
378+
};
379+
let section_trusted_by_default = Safe::directory_filter(section.meta());
380+
if section_trusted_by_default || section.meta().trust == gix_sec::Trust::Full {
381+
continue;
382+
}
383+
let Some(meta_path) = section.meta().path.as_deref() else {
384+
continue;
385+
};
386+
match is_valid_by_path.entry(meta_path.to_owned()) {
387+
Entry::Occupied(entry) => {
388+
if *entry.get() {
389+
section.set_trust(gix_sec::Trust::Full);
390+
} else {
391+
continue;
392+
}
393+
}
394+
Entry::Vacant(entry) => {
395+
let config_file_is_safe = (meta_path.strip_prefix(test_dir).is_ok()
396+
&& *git_dir_trust == gix_sec::Trust::Full)
397+
|| check_safe_directories(
398+
meta_path,
399+
git_install_dir.as_deref(),
400+
current_dir,
401+
home.as_deref(),
402+
&safe_dirs,
403+
)
404+
.is_ok();
405+
406+
entry.insert(config_file_is_safe);
407+
if config_file_is_safe {
408+
section.set_trust(gix_sec::Trust::Full);
409+
}
410+
}
411+
}
412+
}
413+
config.resolved = resolved.into();
414+
}
415+
387416
refs.write_reflog = config::cache::util::reflog_or_default(config.reflog, worktree_dir.is_some());
388417
refs.namespace.clone_from(&config.refs_namespace);
389418
let prefix = replacement_objects_refs_prefix(&config.resolved, lenient_config, filter_config_section)?;

gix/src/repository/location.rs

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ impl crate::Repository {
1212
}
1313

1414
/// The trust we place in the git-dir, with lower amounts of trust causing access to configuration to be limited.
15+
/// Note that if the git-dir is trusted but the worktree is not, the result is that the git-dir is also less trusted.
1516
pub fn git_dir_trust(&self) -> gix_sec::Trust {
1617
self.options.git_dir_trust.expect("definitely set by now")
1718
}

0 commit comments

Comments
 (0)