mirror of
https://github.com/git/git.git
synced 2026-09-29 17:49:58 +02:00
Daniel Martí
8989e7aecd
keyring_get() searches with SECRET_SEARCH_LOAD_SECRETS, then passes
secret_item_get_secret() of the first match unchecked to
secret_value_get_text() and secret_value_unref(). As libsecret
documents, that secret can be NULL: the search does not load secrets
of locked items, such as when SECRET_SEARCH_UNLOCK fails to unlock
them, and it ignores errors from loading secrets. The GNOME keyring
daemon also silently leaves out of its reply any item which is locked
or which was deleted after the search matched it, e.g. by a concurrent
"credential erase" from another git process. We then print
secret_value_get_text: assertion 'value' failed
secret_value_unref: assertion 'value != NULL' failed
before git falls back to prompting for the password.
We could keep the flag and load the secret explicitly only when it is
NULL, but SECRET_SEARCH_LOAD_SECRETS is not part of the search call:
libsecret implements it as a separate GetSecrets D-Bus call after
SearchItems. Drop the flag and instead always load the one secret we
use with secret_item_load_secret_sync(), which reports errors. This
takes as many D-Bus calls as before, and leaves a single code path
that runs every time, rather than a fallback that only runs in a rare
race. libsecret's own secret-tool also loads each secret explicitly
after searching.
An inaccessible item now produces a useful error message instead of
the assertion failures, and git still falls back to prompting. The
race needs a concurrent process or a locked keyring to trigger, so
there is no test.
Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Signed-off-by: Junio C Hamano <gitster@pobox.com>