Skip to content

Read git config strings via a config snapshot in LibGit2Repo - #2086

Merged
tyrielv merged 2 commits into
microsoft:masterfrom
tyrielv:tyvella/fix-libgit2-live-config-getstring
Aug 14, 2026
Merged

Read git config strings via a config snapshot in LibGit2Repo#2086
tyrielv merged 2 commits into
microsoft:masterfrom
tyrielv:tyvella/fix-libgit2-live-config-getstring

Conversation

@tyrielv

@tyrielv tyrielv commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Problem

LibGit2Repo.GetConfigString read a git config value with the native
git_config_get_string. It called that function on the live config object
returned by git_repository_config. libgit2 does not allow
git_config_get_string on a live config and fails with get_string called on a live config object. The read then threw LibGit2Exception, so callers
silently fell back to their default value instead of the configured value.

A second, latent defect sat in the same P/Invoke: git_config_get_string
returns a borrowed const char* owned by the config, but it was marshalled
as an out string. The interop marshaller frees an out-string buffer with
CoTaskMemFree — 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

  • Take a git_config_snapshot of the live config and read the string from the
    snapshot, then free the snapshot. git_config_get_string is valid on a
    snapshot config.
  • Change the git_config_get_string binding to return an IntPtr and copy the
    value with Marshal.PtrToStringUTF8, which never frees the borrowed pointer.
    This matches the manual marshalling already used by GitConfigEntry.
  • A missing key still returns null, so the caller default applies without a
    spurious error. git_config_get_bool parses its value rather than returning
    a borrowed pointer, so GetConfigBool is 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 a
string and a bool config value, reads them back through LibGit2Repo, and
asserts a missing key returns null. A unit test cannot cover this because the
unit 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.

…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
tyrielv marked this pull request as ready for review August 13, 2026 18:24
@tyrielv
tyrielv enabled auto-merge August 13, 2026 18:36
@tyrielv
tyrielv merged commit 8b254e1 into microsoft:master Aug 14, 2026
35 checks passed
This was referenced Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants