Read git config strings via a config snapshot in LibGit2Repo - #2086
Merged
tyrielv merged 2 commits intoAug 14, 2026
Merged
Conversation
…Repo git_config_get_string returns a borrowed pointer whose lifetime is tied to the config object, so libgit2 only permits it on a snapshot (read-only) config. LibGit2Repo.GetConfigString called it on the live config returned by git_repository_config, which fails with "get_string called on a live config object". The read then threw LibGit2Exception, and callers silently fell back to their default value instead of honoring the configured setting. Take a git_config_snapshot of the live config and read the string from the snapshot, freeing the snapshot afterward. Also stop marshalling the result as an out string. git_config_get_string returns a borrowed const char* owned by the config; the interop marshaller would free that pointer with CoTaskMemFree, a mismatched-allocator free of memory libgit2 still owns, corrupting the heap. Retrieve the value as an IntPtr and copy it with Marshal.PtrToStringUTF8, which never frees the borrowed pointer. This matches the manual marshalling already used by GitConfigEntry. Not-found still returns null so the documented default applies without a spurious error. git_config_get_bool is unaffected (it parses the value rather than returning a borrowed pointer), so GetConfigBool is left unchanged. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Add LibGit2ConfigTests, a functional test that exercises the real libgit2 (git2.dll) config-read path in LibGit2Repo against a plain on-disk git repository. It creates a temp repo, sets a string and a bool config value, then reads them back through LibGit2Repo and asserts a missing key returns null. No unit test can cover this: the unit tests mock the native layer, so the "get_string called on a live config object" failure only manifests through the real P/Invoke. This test fails before the snapshot/marshalling fix and passes after, guarding the regression. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
marked this pull request as ready for review
August 13, 2026 18:24
tyrielv
enabled auto-merge
August 13, 2026 18:36
Keith Klein (KeithIsSleeping)
approved these changes
Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
LibGit2Repo.GetConfigStringread a git config value with the nativegit_config_get_string. It called that function on the live config objectreturned by
git_repository_config. libgit2 does not allowgit_config_get_stringon a live config and fails withget_string called on a live config object. The read then threwLibGit2Exception, so callerssilently fell back to their default value instead of the configured value.
A second, latent defect sat in the same P/Invoke:
git_config_get_stringreturns a borrowed
const char*owned by the config, but it was marshalledas an
out string. The interop marshaller frees an out-string buffer withCoTaskMemFree— a mismatched-allocator free of memory libgit2 still owns,which corrupts the heap. The live-config error masked this because the call
never returned a string; fixing the first bug makes the call succeed and would
expose the corruption.
Fix
git_config_snapshotof the live config and read the string from thesnapshot, then free the snapshot.
git_config_get_stringis valid on asnapshot config.
git_config_get_stringbinding to return anIntPtrand copy thevalue with
Marshal.PtrToStringUTF8, which never frees the borrowed pointer.This matches the manual marshalling already used by
GitConfigEntry.null, so the caller default applies without aspurious error.
git_config_get_boolparses its value rather than returninga borrowed pointer, so
GetConfigBoolis unchanged.Tests
Adds
LibGit2ConfigTests, a functional test that exercises the real libgit2(
git2.dll) config path against a plain on-disk git repository: it sets astring and a bool config value, reads them back through
LibGit2Repo, andasserts a missing key returns
null. A unit test cannot cover this because theunit tests mock the native layer; the failure only appears through the real
P/Invoke. The test fails before this change and passes after.
Unit suite: 909 passed, 0 failed locally. The functional test was not run
locally (the native C++ projects need a toolset not available on this machine);
the PR validation build runs it.