{"thread":{"id":"65529","subject":"[BUG] git-credential-libsecret writes secret to stdout on store","startedAt":"2026-04-21T11:28:22Z","lastAt":"2026-04-22T13:13:37Z","messageCount":4,"participants":["Lutz-Christian Quander","Mantas Mikulėnas","Phillip Wood"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"542035","messageId":"b7b6b94c-7e42-42a5-95e5-d44a54d6da0f@wateringcan.de","threadId":"65529","inReplyTo":null,"subject":"[BUG] git-credential-libsecret writes secret to stdout on store","fromName":"Lutz-Christian Quander","fromEmail":"lcq@wateringcan.de","sentAt":"2026-04-21T11:03:40Z","receivedAt":"2026-04-21T11:28:22Z","isPatch":false,"body":"Hello,\n\nI believe I've hit a bug in contrib/credential/libsecret that leaks\nthe secret to stdout on `store` (and, by inspection of the same code\npath, `erase`). It reproduces on the current 2.53.0 Arch package and\nthe relevant code path is unchanged on master as of today.\n\nSummary\n-------\n\n`git-credential-libsecret store` unconditionally echoes the\n`username` and `password` from its parsed input back to stdout after\nthe store operation completes. This exposes the secret to whatever\nconsumes the helper's stdout -- terminal scrollback, shell\npipelines, CI logs, or any parent-process capture -- whenever a\ncaller feeds credentials in via pipe (the documented non-interactive\nseeding pattern).\n\n`get` should write credentials to stdout. `store` and `erase` should\nnot.\n\nAffected version\n----------------\n\n- Reproduced on git 2.53.0-1.1 (Arch Linux community/git).\n- Source inspected from the installed package\n   (/usr/share/git/credential/libsecret/git-credential-libsecret.c)\n   and confirmed against\n   contrib/credential/libsecret/git-credential-libsecret.c on\n   origin/master.\n\nReproduction\n------------\n\n     $ printf \n\"protocol=https\\nhost=example.invalid\\nusername=alice\\npassword=SECRET123\\n\\n\" \n| /usr/lib/git-core/git-credential-libsecret store\n     username=alice\n     password=SECRET123\n     $ echo $?\n     0\n\nOnly `username=` and `password=` are echoed (not `protocol=` /\n`host=`), matching the four fields `credential_write()` emits.\n\nExpected behaviour\n------------------\n\n`store` produces no stdout on success. Exit code unchanged.\n\nRoot cause\n----------\n\nIn main(), the write is unconditional after the op dispatch:\n\n     ret = credential_read(&cred);\n     if (ret)\n             goto out;\n\n     /* perform credential operation */\n     ret = (*try_op->op)(&cred);\n\n     credential_write(&cred);   /* unconditional for get/store/erase */\n\n`credential_write()` emits `username`, `password`,\n`password_expiry_utc`, and `oauth_refresh_token` to stdout. That is\ncorrect for `get` (returning the looked-up credential) and incorrect\nfor `store` / `erase`, where the struct still holds the just-read\nstdin input.\n\nComparable helpers in the same tree should probably be audited for\nthe same pattern; at least `credential-store` historically only\nwrites on `get`.\n\nProposed fix\n------------\n\nMinimal change -- guard the write:\n\n         ret = credential_read(&cred);\n         if (ret)\n             goto out;\n\n         /* perform credential operation */\n         ret = (*try_op->op)(&cred);\n\n     -    credential_write(&cred);\n     +    if (!strcmp(argv[1], \"get\"))\n     +        credential_write(&cred);\n\nA cleaner refactor would add a `writes_output` flag or a\n`write_result` callback to `struct credential_operation` so each op\ndeclares its own output contract, but the guard above is the\nsmallest safe change. Happy to turn it into a proper patch with\nsign-off if that's preferred.\n\nSecurity impact\n---------------\n\nThe documented pattern for seeding credentials non-interactively is:\n\n     printf \"protocol=...\\nhost=...\\nusername=...\\npassword=...\\n\\n\" | \ngit-credential-<helper> store\n\nRunning this at a terminal prints the secret into scrollback.\nRunning it in a shell script whose stdout goes to a log file\npersists the secret in that log. Running it in CI captures the\nsecret in the pipeline artefact. Every real-world use of the\ndocumented pattern is affected.\n\nSeverity is moderate: the leak requires the user to run a legitimate\ncommand -- no attacker-controlled input path -- but the leak happens\non the \"correct\" documented workflow, silently, with exit code 0.\n\nWorkaround\n----------\n\nRedirect stdout explicitly:\n\n     printf \"...\\n\" | /usr/lib/git-core/git-credential-libsecret store \n >/dev/null\n\nEnvironment\n-----------\n\n- Distribution: CachyOS (Arch Linux derivative)\n- Kernel: Linux 7.0.0-1-cachyos\n- git: 2.53.0-1.1\n- Shell: bash (invoked from a fish login shell)\n\nHappy to coordinate disclosure if preferred, but the workaround is\ntrivial, the patch is one line, and the bug affects any scripted\ncredential seeding -- so there's little to gain from embargo.\n\nThanks,\n\nLutz-Christian Quander\n\n"},{"id":"542036","messageId":"2d5b37b0-3442-42f8-81f4-18b48e95a617@gmail.com","threadId":"65529","inReplyTo":"b7b6b94c-7e42-42a5-95e5-d44a54d6da0f@wateringcan.de","subject":"Re: [BUG] git-credential-libsecret writes secret to stdout on store","fromName":"Mantas Mikulėnas","fromEmail":"grawity@gmail.com","sentAt":"2026-04-21T11:37:56Z","receivedAt":"2026-04-21T11:38:00Z","isPatch":false,"body":"On 21/04/2026 14.03, Lutz-Christian Quander wrote:\n> The documented pattern for seeding credentials non-interactively is:\n>\n>     printf \"protocol=...\\nhost=...\\nusername=...\\npassword=...\\n\\n\" | \n> git-credential-<helper> store\n>\n> Running this at a terminal prints the secret into scrollback.\n> Running it in a shell script whose stdout goes to a log file\n> persists the secret in that log. Running it in CI captures the\n> secret in the pipeline artefact. Every real-world use of the\n> documented pattern is affected.\n>\n> Severity is moderate: the leak requires the user to run a legitimate\n> command -- no attacker-controlled input path -- but the leak happens\n> on the \"correct\" documented workflow, silently, with exit code 0.\n\nIs it actually the correct documented workflow? I couldn't find it in \nthe Git docs. My understanding was that writing to \"git credential \napprove\" was the sole user interface, while \"git-credential-<helper> \nstore\" was the internal interface between the git-credential builtin and \nthe helper.\n\n"},{"id":"542095","messageId":"0b2370ed-f3e1-4011-8a2c-8da539759881@gmail.com","threadId":"65529","inReplyTo":"60cf5f7c-9ccb-4dfe-82e4-9b6e54b3c2c0@wateringcan.de","subject":"Re: [BUG] git-credential-libsecret writes secret to stdout on store","fromName":"Mantas Mikulėnas","fromEmail":"grawity@gmail.com","sentAt":"2026-04-22T05:49:25Z","receivedAt":"2026-04-22T05:49:29Z","isPatch":false,"body":"(Re-adding list to recipients.)\n\nOn 21/04/2026 14.47, Lutz-Christian Quander wrote:\n> Thanks — fair correction. I tested `git credential approve` with the\n> libsecret helper and it does not leak: git discards the helper's\n> stdout on `store`, so the documented user-facing interface is safe.\n> The leak only manifests when the helper binary is invoked directly.\n>\n> That narrows the argument, but I'd still submit that the fix is\n> worth landing for two reasons:\n>\n> 1. gitcredentials(7) specifies that helpers should use stderr (not\n>    stdout) for messages on `store`/`erase`, and that helper stdout\n>    is ignored on those operations. The current unconditional\n>    `credential_write()` violates that contract regardless of how the\n>    helper is invoked -- it just happens to be harmless when git is\n>    the caller because git discards the stream.\n\nThe API contract isn't violated IMO, as documenting \"output is ignored\" \ngives permission to produce output, even if that output is unnecessary. \n(that is, from my reading, it implies that output *will* go to /dev/null \n– and running the helper manually is what really violates the API \ncontract from the other side by not ignoring the helper's output...)\n\nI agree that the unconditional credential_write() is a bit weird upon a \ncloser look – it and the entire main() was just copied as-is from the \nolder \"gnomekeyring\" helper, under assumption that that was \"the \nexpected way\" the credential_*() functions were to be used.\n\n\n>\n> 2. The direct-invocation pattern shows up widely in distro docs,\n>    StackOverflow answers, and automation scripts -- empirically the\n>    \"internal protocol\" boundary is porous. Fixing the helper is one\n>    line; documenting the internal boundary across the ecosystem is\n>    not.\n>\n> If the preferred answer is instead \"users should only use\n> `git credential approve`\", that would also work for me, but it may\n> deserve a note in gitcredentials(7) to steer people away from the\n> direct pattern -- the current docs don't actively discourage it.\n\n\nI think it should be fixed to remove the useless output, especially if \nthe command is as widely documented as you say (although I'm actually \nsurprised that any CI environments even have a libsecret backend running \n*in the first place*; I would have assumed that they would use \ngit-credential-cache or something instead). You should send a patch.\n\nAt the same time, I also think it's a bit too much 'self-inflicted' of a \nsecurity issue to be CVE-worthy, so to speak... I mean, I don't like \nautomatically blaming the user for holding it wrong, but in this \nparticular case, I'd like to think that one would test the command on \ntheir own machine first and see how it behaves before putting it in a \nscript.\n\n\n>\n> Happy with whichever direction you prefer.\n>\n> Best regards\n>\n> Lutz-Christian Quander\n>\n> p.s.\n>\n> Thank you for your very quick reponse and your Open Source work\n>\n>\n> Am 21.04.26 um 13:37 schrieb Mantas Mikulėnas:\n>> On 21/04/2026 14.03, Lutz-Christian Quander wrote:\n>>> The documented pattern for seeding credentials non-interactively is:\n>>>\n>>>     printf \"protocol=...\\nhost=...\\nusername=...\\npassword=...\\n\\n\" \n>>> | git-credential-<helper> store\n>>>\n>>> Running this at a terminal prints the secret into scrollback.\n>>> Running it in a shell script whose stdout goes to a log file\n>>> persists the secret in that log. Running it in CI captures the\n>>> secret in the pipeline artefact. Every real-world use of the\n>>> documented pattern is affected.\n>>>\n>>> Severity is moderate: the leak requires the user to run a legitimate\n>>> command -- no attacker-controlled input path -- but the leak happens\n>>> on the \"correct\" documented workflow, silently, with exit code 0.\n>>\n>> Is it actually the correct documented workflow? I couldn't find it in \n>> the Git docs. My understanding was that writing to \"git credential \n>> approve\" was the sole user interface, while \"git-credential-<helper> \n>> store\" was the internal interface between the git-credential builtin \n>> and the helper.\n>>\n"},{"id":"542127","messageId":"fe75e0a5-1a87-4515-b02b-bfbef0366aaf@gmail.com","threadId":"65529","inReplyTo":"0b2370ed-f3e1-4011-8a2c-8da539759881@gmail.com","subject":"Re: [BUG] git-credential-libsecret writes secret to stdout on store","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-22T13:13:33Z","receivedAt":"2026-04-22T13:13:37Z","isPatch":false,"body":"On 22/04/2026 06:49, Mantas Mikulėnas wrote:\n>> 2. The direct-invocation pattern shows up widely in distro docs,\n>>    StackOverflow answers, and automation scripts -- empirically the\n>>    \"internal protocol\" boundary is porous. Fixing the helper is one\n>>    line; documenting the internal boundary across the ecosystem is\n>>    not.\n>>\n>> If the preferred answer is instead \"users should only use\n>> `git credential approve`\", that would also work for me, but it may\n>> deserve a note in gitcredentials(7) to steer people away from the\n>> direct pattern -- the current docs don't actively discourage it.\n\nYes, users should be using \"git credential\", not be running the helpers \ndirectly. That's why the helpers are installed in a directory that is \nnot in $PATH. gitcredentials(7) shows how to set the config setting used \nby \"git credential\", as far as I can see it does not suggest that users \nshould be running the helpers directly.\n\nThanks\n\nPhillip\n\n"}]}