Git development
 help / color / mirror / Atom feed
* [BUG] git-credential-libsecret writes secret to stdout on store
@ 2026-04-21 11:03 Lutz-Christian Quander
  2026-04-21 11:37 ` Mantas Mikulėnas
  0 siblings, 1 reply; 4+ messages in thread
From: Lutz-Christian Quander @ 2026-04-21 11:03 UTC (permalink / raw)
  To: git; +Cc: grawity

Hello,

I believe I've hit a bug in contrib/credential/libsecret that leaks
the secret to stdout on `store` (and, by inspection of the same code
path, `erase`). It reproduces on the current 2.53.0 Arch package and
the relevant code path is unchanged on master as of today.

Summary
-------

`git-credential-libsecret store` unconditionally echoes the
`username` and `password` from its parsed input back to stdout after
the store operation completes. This exposes the secret to whatever
consumes the helper's stdout -- terminal scrollback, shell
pipelines, CI logs, or any parent-process capture -- whenever a
caller feeds credentials in via pipe (the documented non-interactive
seeding pattern).

`get` should write credentials to stdout. `store` and `erase` should
not.

Affected version
----------------

- Reproduced on git 2.53.0-1.1 (Arch Linux community/git).
- Source inspected from the installed package
   (/usr/share/git/credential/libsecret/git-credential-libsecret.c)
   and confirmed against
   contrib/credential/libsecret/git-credential-libsecret.c on
   origin/master.

Reproduction
------------

     $ printf 
"protocol=https\nhost=example.invalid\nusername=alice\npassword=SECRET123\n\n" 
| /usr/lib/git-core/git-credential-libsecret store
     username=alice
     password=SECRET123
     $ echo $?
     0

Only `username=` and `password=` are echoed (not `protocol=` /
`host=`), matching the four fields `credential_write()` emits.

Expected behaviour
------------------

`store` produces no stdout on success. Exit code unchanged.

Root cause
----------

In main(), the write is unconditional after the op dispatch:

     ret = credential_read(&cred);
     if (ret)
             goto out;

     /* perform credential operation */
     ret = (*try_op->op)(&cred);

     credential_write(&cred);   /* unconditional for get/store/erase */

`credential_write()` emits `username`, `password`,
`password_expiry_utc`, and `oauth_refresh_token` to stdout. That is
correct for `get` (returning the looked-up credential) and incorrect
for `store` / `erase`, where the struct still holds the just-read
stdin input.

Comparable helpers in the same tree should probably be audited for
the same pattern; at least `credential-store` historically only
writes on `get`.

Proposed fix
------------

Minimal change -- guard the write:

         ret = credential_read(&cred);
         if (ret)
             goto out;

         /* perform credential operation */
         ret = (*try_op->op)(&cred);

     -    credential_write(&cred);
     +    if (!strcmp(argv[1], "get"))
     +        credential_write(&cred);

A cleaner refactor would add a `writes_output` flag or a
`write_result` callback to `struct credential_operation` so each op
declares its own output contract, but the guard above is the
smallest safe change. Happy to turn it into a proper patch with
sign-off if that's preferred.

Security impact
---------------

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.

Workaround
----------

Redirect stdout explicitly:

     printf "...\n" | /usr/lib/git-core/git-credential-libsecret store 
 >/dev/null

Environment
-----------

- Distribution: CachyOS (Arch Linux derivative)
- Kernel: Linux 7.0.0-1-cachyos
- git: 2.53.0-1.1
- Shell: bash (invoked from a fish login shell)

Happy to coordinate disclosure if preferred, but the workaround is
trivial, the patch is one line, and the bug affects any scripted
credential seeding -- so there's little to gain from embargo.

Thanks,

Lutz-Christian Quander


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [BUG] git-credential-libsecret writes secret to stdout on store
  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>
  0 siblings, 1 reply; 4+ messages in thread
From: Mantas Mikulėnas @ 2026-04-21 11:37 UTC (permalink / raw)
  To: Lutz-Christian Quander, git

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.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [BUG] git-credential-libsecret writes secret to stdout on store
       [not found]   ` <60cf5f7c-9ccb-4dfe-82e4-9b6e54b3c2c0@wateringcan.de>
@ 2026-04-22  5:49     ` Mantas Mikulėnas
  2026-04-22 13:13       ` Phillip Wood
  0 siblings, 1 reply; 4+ messages in thread
From: Mantas Mikulėnas @ 2026-04-22  5:49 UTC (permalink / raw)
  To: Lutz-Christian Quander; +Cc: git

(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.
>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [BUG] git-credential-libsecret writes secret to stdout on store
  2026-04-22  5:49     ` Mantas Mikulėnas
@ 2026-04-22 13:13       ` Phillip Wood
  0 siblings, 0 replies; 4+ messages in thread
From: Phillip Wood @ 2026-04-22 13:13 UTC (permalink / raw)
  To: Mantas Mikulėnas, Lutz-Christian Quander; +Cc: git

On 22/04/2026 06:49, Mantas Mikulėnas wrote:
>> 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.

Yes, users should be using "git credential", not be running the helpers 
directly. That's why the helpers are installed in a directory that is 
not in $PATH. gitcredentials(7) shows how to set the config setting used 
by "git credential", as far as I can see it does not suggest that users 
should be running the helpers directly.

Thanks

Phillip


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-22 13:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-04-22 13:13       ` Phillip Wood

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox