Skip to content

The monthly report for April 2025 #1965

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 2 commits into from
Apr 22, 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
47 changes: 47 additions & 0 deletions etc/reports/25-04.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
While the previous month was full of bug reports, this month is full of fixes (fortunately). And there is more….

## SHA1 with collision detection

Let's face it, `gitoxide` had it coming. Using the fastest possible SHA1 implementation was great while not too many people were using it, but with `jj` using it for Git interactions the requirements changed.

It was quite a lot of [contributed work](https://github.com/GitoxideLabs/gitoxide/pull/1915), but now we are finally en-par with Git, which seems to also use a slow SHA1 implementation with collision detection. Unfortunately, this also removes a major performance advantage as it reduces the SHA1 hashing speed by a factor of roughly 3x, a slowdown most noticeable when cloning repositories. In numbers, on my machine I could previously get ~24.5GB/s hashing speed on, and now it's down to 8.5GB/s.

On the bright side, this will definitely increase the motivation of supporting SHA256 sooner, which should hash with up to 2.1GB per core on my machine, nearly twice as fast as SHA1 prior to collision detection.

Finally, let me share the [security advisory](https://github.com/GitoxideLabs/gitoxide/security/advisories/GHSA-2frx-2596-x5r6) that motivated the fix, which should help the downstream to upgrade to the latest release.

## `zlib-rs` as the one and only

With [zlib-rs](https://github.com/trifectatechfoundation/zlib-rs) we now have a pure-Rust implementation of `zlib-ng`, which for `gitoxide` turned out to be 1% faster than `zlib-ng`, the previously fastest C implementation. That number is an even greater achievement when realizing this is 1% of 3x longer time compared to the SHA1 implementation without collision detection.

Thanks to [Josh Triplett](https://github.com/joshtriplett/) we could now deprecate all zlib-related configuration flags as a pure Rust implementation will never clash with C symbol exports. In a future step, the deprecated Cargo features will be removed for a great reduction in configuration complexity.

## Improved `safe.directory` handling

As `gitoxide` had the luxury of implementing Git from the ground up it could prepare for certain 'concepts' while Git had to tack them on. With that in mind, `safe.directory` wasn't all that important, to the point where it wasn't used where it should have been.

For one, Git now supports `*` as wildcard-suffixes, allowing to set a directory prefix as safe, and with [this fix](https://github.com/GitoxideLabs/gitoxide/issues/1912) `gitoxide` will do the same. This will also affect Git configuration, which previously wasn't trusted as `safe.directory` didn't affect it.

Additionally, and unfortunately only with manual testing, I now believe that the fixed implementation is similar to what Git does.

## Welcome to the Family

In an effort to unify the maintenance setup of various related projects, as driven by [Eliah Kagan](https://github.com/EliahKagan), we brought `prodash`, `cargo-smart-release` and `learning-Rust-with-Gitoxide` into the GitoxideLabs organization.
From here they will get more timely security updates and patches, and maybe even some improvements here and there.

## Community

### An even faster "Blame" with caching

Even though it's not quite done, yet, I thought it was worth highlighting that Fractional has been working on [adding caching support](https://github.com/GitoxideLabs/gitoxide/pull/1852) for `gix blame`, with examples showing a 80x speedup!
When merged, `gitoxide` will be able to support a new generation of investigative tooling that quickly tells users where lines have been changed, and more.

### Gix in Cargo

Thanks to the generous support of [the Rustweek organisers](https://rustweek.org/unconf-intro/) Christoph Rüßler and me will have the opportunity to join the Critical Infrastructure group of the [Unconference](https://rustweek.org/unconf/). There, I hope to get in touch with users of Gitoxide, and of course, the Cargo team to finally meet in person :). And of course, I'd love to finish the [STF application](https://github.com/GitoxideLabs/gitoxide/issues/1406) to get a grant supporting `gitoxide` development, which ideally will be beneficial for `zlib-rs` as well.
With a grant, feature development could be sped up, and I could work on `gitoxide` more as well.

Cheers
Sebastian

PS: The latest timesheets can be found [here (2025)](https://github.com/Byron/byron/blob/main/timesheets/2025.csv).
135 changes: 82 additions & 53 deletions gix/src/open/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use crate::{
use gix_features::threading::OwnShared;
use gix_object::bstr::ByteSlice;
use gix_path::RelativePath;
use std::collections::btree_map::Entry;
use std::collections::BTreeMap;
use std::ffi::OsStr;
use std::{borrow::Cow, path::PathBuf};

Expand Down Expand Up @@ -157,7 +159,7 @@ impl ThreadSafeRepository {
) -> Result<Self, Error> {
let _span = gix_trace::detail!("open_from_paths()");
let Options {
git_dir_trust,
ref mut git_dir_trust,
object_store_slots,
filter_config_section,
lossy_config,
Expand All @@ -174,15 +176,15 @@ impl ThreadSafeRepository {
ref cli_config_overrides,
ref mut current_dir,
} = options;
let git_dir_trust = git_dir_trust.expect("trust must be determined by now");
let git_dir_trust = git_dir_trust.as_mut().expect("trust must be determined by now");

let mut common_dir = gix_discover::path::from_plain_file(git_dir.join("commondir").as_ref())
.transpose()?
.map(|cd| git_dir.join(cd));
let repo_config = config::cache::StageOne::new(
common_dir.as_deref().unwrap_or(&git_dir),
git_dir.as_ref(),
git_dir_trust,
*git_dir_trust,
lossy_config,
lenient_config,
)?;
Expand Down Expand Up @@ -248,56 +250,6 @@ impl ThreadSafeRepository {
cli_config_overrides,
)?;

// 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
if !config.is_bare {
let mut key_source = None;
Expand Down Expand Up @@ -384,6 +336,83 @@ impl ThreadSafeRepository {
}
}

// 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
|| worktree_dir
.as_deref()
.is_some_and(|wd| !gix_sec::identity::is_path_owned_by_current_user(wd).unwrap_or(false))
{
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();
let test_dir = worktree_dir.as_deref().unwrap_or(git_dir.as_path());
let res = check_safe_directories(
test_dir,
git_install_dir.as_deref(),
current_dir,
home.as_deref(),
&safe_dirs,
);
if res.is_ok() {
*git_dir_trust = gix_sec::Trust::Full;
} else if bail_if_untrusted {
res?;
} else {
// This is how the worktree-trust can reduce the git-dir trust.
*git_dir_trust = gix_sec::Trust::Reduced;
}

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();
let mut is_valid_by_path = BTreeMap::new();
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;
};
match is_valid_by_path.entry(meta_path.to_owned()) {
Entry::Occupied(entry) => {
if *entry.get() {
section.set_trust(gix_sec::Trust::Full);
} else {
continue;
}
}
Entry::Vacant(entry) => {
let config_file_is_safe = (meta_path.strip_prefix(test_dir).is_ok()
&& *git_dir_trust == gix_sec::Trust::Full)
|| check_safe_directories(
meta_path,
git_install_dir.as_deref(),
current_dir,
home.as_deref(),
&safe_dirs,
)
.is_ok();

entry.insert(config_file_is_safe);
if config_file_is_safe {
section.set_trust(gix_sec::Trust::Full);
}
}
}
}
config.resolved = resolved.into();
}

refs.write_reflog = config::cache::util::reflog_or_default(config.reflog, worktree_dir.is_some());
refs.namespace.clone_from(&config.refs_namespace);
let prefix = replacement_objects_refs_prefix(&config.resolved, lenient_config, filter_config_section)?;
Expand Down
1 change: 1 addition & 0 deletions gix/src/repository/location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ impl crate::Repository {
}

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