Git development
 help / color / mirror / Atom feed
From: "Mantas Mikulėnas" <grawity@gmail.com>
To: Lutz-Christian Quander <lcq@wateringcan.de>
Cc: git@vger.kernel.org
Subject: Re: [BUG] git-credential-libsecret writes secret to stdout on store
Date: Wed, 22 Apr 2026 08:49:25 +0300	[thread overview]
Message-ID: <0b2370ed-f3e1-4011-8a2c-8da539759881@gmail.com> (raw)
In-Reply-To: <60cf5f7c-9ccb-4dfe-82e4-9b6e54b3c2c0@wateringcan.de>

(Re-adding list to recipients.)

On 21/04/2026 14.47, Lutz-Christian Quander wrote:
> Thanks — fair correction. I tested `git credential approve` with the
> libsecret helper and it does not leak: git discards the helper's
> stdout on `store`, so the documented user-facing interface is safe.
> The leak only manifests when the helper binary is invoked directly.
>
> That narrows the argument, but I'd still submit that the fix is
> worth landing for two reasons:
>
> 1. gitcredentials(7) specifies that helpers should use stderr (not
>    stdout) for messages on `store`/`erase`, and that helper stdout
>    is ignored on those operations. The current unconditional
>    `credential_write()` violates that contract regardless of how the
>    helper is invoked -- it just happens to be harmless when git is
>    the caller because git discards the stream.

The API contract isn't violated IMO, as documenting "output is ignored" 
gives permission to produce output, even if that output is unnecessary. 
(that is, from my reading, it implies that output *will* go to /dev/null 
– and running the helper manually is what really violates the API 
contract from the other side by not ignoring the helper's output...)

I agree that the unconditional credential_write() is a bit weird upon a 
closer look – it and the entire main() was just copied as-is from the 
older "gnomekeyring" helper, under assumption that that was "the 
expected way" the credential_*() functions were to be used.


>
> 2. The direct-invocation pattern shows up widely in distro docs,
>    StackOverflow answers, and automation scripts -- empirically the
>    "internal protocol" boundary is porous. Fixing the helper is one
>    line; documenting the internal boundary across the ecosystem is
>    not.
>
> If the preferred answer is instead "users should only use
> `git credential approve`", that would also work for me, but it may
> deserve a note in gitcredentials(7) to steer people away from the
> direct pattern -- the current docs don't actively discourage it.


I think it should be fixed to remove the useless output, especially if 
the command is as widely documented as you say (although I'm actually 
surprised that any CI environments even have a libsecret backend running 
*in the first place*; I would have assumed that they would use 
git-credential-cache or something instead). You should send a patch.

At the same time, I also think it's a bit too much 'self-inflicted' of a 
security issue to be CVE-worthy, so to speak... I mean, I don't like 
automatically blaming the user for holding it wrong, but in this 
particular case, I'd like to think that one would test the command on 
their own machine first and see how it behaves before putting it in a 
script.


>
> Happy with whichever direction you prefer.
>
> Best regards
>
> Lutz-Christian Quander
>
> p.s.
>
> Thank you for your very quick reponse and your Open Source work
>
>
> Am 21.04.26 um 13:37 schrieb Mantas Mikulėnas:
>> On 21/04/2026 14.03, Lutz-Christian Quander wrote:
>>> The documented pattern for seeding credentials non-interactively is:
>>>
>>>     printf "protocol=...\nhost=...\nusername=...\npassword=...\n\n" 
>>> | git-credential-<helper> store
>>>
>>> Running this at a terminal prints the secret into scrollback.
>>> Running it in a shell script whose stdout goes to a log file
>>> persists the secret in that log. Running it in CI captures the
>>> secret in the pipeline artefact. Every real-world use of the
>>> documented pattern is affected.
>>>
>>> Severity is moderate: the leak requires the user to run a legitimate
>>> command -- no attacker-controlled input path -- but the leak happens
>>> on the "correct" documented workflow, silently, with exit code 0.
>>
>> Is it actually the correct documented workflow? I couldn't find it in 
>> the Git docs. My understanding was that writing to "git credential 
>> approve" was the sole user interface, while "git-credential-<helper> 
>> store" was the internal interface between the git-credential builtin 
>> and the helper.
>>

  parent reply	other threads:[~2026-04-22  5:49 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-21 11:03 [BUG] git-credential-libsecret writes secret to stdout on store Lutz-Christian Quander
2026-04-21 11:37 ` Mantas Mikulėnas
     [not found]   ` <60cf5f7c-9ccb-4dfe-82e4-9b6e54b3c2c0@wateringcan.de>
2026-04-22  5:49     ` Mantas Mikulėnas [this message]
2026-04-22 13:13       ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0b2370ed-f3e1-4011-8a2c-8da539759881@gmail.com \
    --to=grawity@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=lcq@wateringcan.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox