With the recent tightening of the code that is used to parse
various parts of a URL for use in the credential subsystem, a
hand-edited credential-store file causes the credential helper to
die, which is a bit too harsh to the users. Demote the error
behaviour to just ignore and keep using well-formed lines instead.
* cb/credential-store-ignore-bogus-lines:
credential-store: ignore bogus lines from store file
credential-store: document the file format a bit more
With the added checks for invalid URLs in credentials, any locally
modified store files which might have empty lines or even comments
were reported[1] failing to parse as valid credentials.
Instead of doing a hard check for credentials, do a soft one and
therefore avoid the reported fatal error.
While at it add tests for all known corruptions that are currently
ignored to keep track of them and avoid the risk of regressions.
[1] https://stackoverflow.com/a/61420852/5005936
Reported-by: Dirk <dirk@ed4u.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Based-on-patch-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allow callers to specify exactly what characters need to be URL-encoded
and which do not. This new API will be taken advantage of in a patch
later in this set.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Matthew DeVore <matvore@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the next patch we need the ability to expand '~' to
real_path($HOME). But we can't do that from outside because '~' is part
of a pattern, not a true path. Add an option to expand_user_path() to do
so.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are certain startup tasks that we expect every git
process to do. In some cases this is just to improve the
quality of the program (e.g., setting up gettext()). In
others it is a requirement for using certain functions in
libgit.a (e.g., system_path() expects that you have called
git_extract_argv0_path()).
Most commands are builtins and are covered by the git.c
version of main(). However, there are still a few external
commands that use their own main(). Each of these has to
remember to include the correct startup sequence, and we are
not always consistent.
Rather than just fix the inconsistencies, let's make this
harder to get wrong by providing a common main() that can
run this standard startup.
We basically have two options to do this:
- the compat/mingw.h file already does something like this by
adding a #define that replaces the definition of main with a
wrapper that calls mingw_startup().
The upside is that the code in each program doesn't need
to be changed at all; it's rewritten on the fly by the
preprocessor.
The downside is that it may make debugging of the startup
sequence a bit more confusing, as the preprocessor is
quietly inserting new code.
- the builtin functions are all of the form cmd_foo(),
and git.c's main() calls them.
This is much more explicit, which may make things more
obvious to somebody reading the code. It's also more
flexible (because of course we have to figure out _which_
cmd_foo() to call).
The downside is that each of the builtins must define
cmd_foo(), instead of just main().
This patch chooses the latter option, preferring the more
explicit approach, even though it is more invasive. We
introduce a new file common-main.c, with the "real" main. It
expects to call cmd_main() from whatever other objects it is
linked against.
We link common-main.o against anything that links against
libgit.a, since we know that such programs will need to do
this setup. Note that common-main.o can't actually go inside
libgit.a, as the linker would not pick up its main()
function automatically (it has no callers).
The rest of the patch is just adjusting all of the various
external programs (mostly in t/helper) to use cmd_main().
I've provided a global declaration for cmd_main(), which
means that all of the programs also need to match its
signature. In particular, many functions need to switch to
"const char **" instead of "char **" for argv. This effect
ripples out to a few other variables and functions, as well.
This makes the patch even more invasive, but the end result
is much better. We should be treating argv strings as const
anyway, and now all programs conform to the same signature
(which also matches the way builtins are defined).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The strbuf_getline() interface allows a byte other than LF or NUL as
the line terminator, but this is only because I wrote these
codepaths anticipating that there might be a value other than NUL
and LF that could be useful when I introduced line_termination long
time ago. No useful caller that uses other value has emerged.
By now, it is clear that the interface is overly broad without a
good reason. Many codepaths have hardcoded preference to read
either LF terminated or NUL terminated records from their input, and
then call strbuf_getline() with LF or NUL as the third parameter.
This step introduces two thin wrappers around strbuf_getline(),
namely, strbuf_getline_lf() and strbuf_getline_nul(), and
mechanically rewrites these call sites to call either one of
them. The changes contained in this patch are:
* introduction of these two functions in strbuf.[ch]
* mechanical conversion of all callers to strbuf_getline() with
either '\n' or '\0' as the third parameter to instead call the
respective thin wrapper.
After this step, output from "git grep 'strbuf_getline('" would
become a lot smaller. An interim goal of this series is to make
this an empty set, so that we can have strbuf_getline_crlf() take
over the shorter name strbuf_getline().
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The error message after a failing commit_lock_file() call sometimes
looks like this, causing confusion:
$ git remote add remote git@server.com/repo.git
error: could not commit config file .git/config
# Huh?!
# I didn't want to commit anything, especially not my config file!
While in the narrow context of the lockfile module using the verb
'commit' in the error message makes perfect sense, in the broader
context of git the word 'commit' already has a very specific meaning,
hence the confusion.
Reword these error messages to say "could not write" instead of "could
not commit".
While at it, include strerror in the error messages after writing the
config file or the credential store fails to provide some information
about the cause of the failure, and update the style of the error
message after writing the reflog fails to match surrounding error
messages (i.e. no '' around the pathname and no () around the error
description).
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Jeff King <peff@peff.net>
We are about to move those members, so change client code to read them
through accessor functions.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code clean-up for xdg configuration path support.
* pt/xdg-config-path:
path.c: remove home_config_paths()
git-config: replace use of home_config_paths()
git-commit: replace use of home_config_paths()
credential-store.c: replace home_config_paths() with xdg_config_home()
dir.c: replace home_config_paths() with xdg_config_home()
attr.c: replace home_config_paths() with xdg_config_home()
path.c: implement xdg_config_home()
Tweak the sample "store" backend of the credential helper to honor
XDG configuration file locations when specified.
* pt/credential-xdg:
t0302: "unreadable" test needs POSIXPERM
t0302: test credential-store support for XDG_CONFIG_HOME
git-credential-store: support XDG_CONFIG_HOME
git-credential-store: support multiple credential files
Since only the xdg credentials file path is required, and
home_config_paths() is unable to construct the path ~/.git-credentials,
simplify the code by replacing home_config_paths() with
xdg_config_home().
Signed-off-by: Paul Tan <pyokagan@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add $XDG_CONFIG_HOME/git/credentials to the default credential search
path of git-credential-store. This allows git-credential-store to
support user-specific configuration files in accordance with the XDG
base directory specification[1].
[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-0.7.html
~/.git-credentials has a higher precedence than
$XDG_CONFIG_HOME/git/credentials when looking up credentials. This
means that if any duplicate matching credentials are found in the xdg
file (due to ~/.git-credentials being updated by old versions of git or
outdated tools), they will not be used at all. This is to give the user
some leeway in switching to old versions of git while keeping the xdg
directory. This is consistent with the behavior of git-config.
However, the higher precedence of ~/.git-credentials means that as long
as ~/.git-credentials exist, all credentials will be written to the
~/.git-credentials file even if the user has an xdg file as having a
~/.git-credentials file indicates that the user wants to preserve
backwards-compatibility. This is also consistent with the behavior of
git-config.
To make this precedence explicit in docs/git-credential-store, add a new
section FILES that lists out the credential file paths in their order of
precedence, and explain how the ordering affects the lookup, storage and
erase operations.
Also, update the documentation for --file to briefly explain the
operations on multiple files if the --file option is not provided.
Since the xdg file will not be used unless it actually exists, to
prevent the situation where some credentials are present in the xdg file
while some are present in the home file, users are recommended to not
create the xdg file if they require compatibility with old versions of
git or outdated tools. Note, though, that "erase" can be used to
explicitly erase matching credentials from all files.
Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previously, git-credential-store only supported storing credentials in a
single file: ~/.git-credentials. In order to support the XDG base
directory specification[1], git-credential-store needs to be able to
lookup and erase credentials from multiple files, as well as to pick the
appropriate file to write to so that the credentials can be found on
subsequent lookups.
[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-0.7.html
Note that some credential storage files may not be owned, readable or
writable by the user, as they may be system-wide files that are meant to
apply to every user.
Instead of a single file path, lookup_credential(), remove_credential()
and store_credential() now take a precedence-ordered string_list of
file paths. lookup_credential() expects both user-specific and
system-wide credential files to be provided to support the use case of
system administrators setting default credentials for users.
remove_credential() and store_credential() expect only the user-specific
credential files to be provided as usually the only config files that
users are allowed to edit are their own user-specific ones.
lookup_credential() will read these (user-specific and system-wide) file
paths in order until it finds the 1st matching credential and print it.
As some files may be private and thus unreadable, any file which cannot
be read will be ignored silently.
remove_credential() will erase credentials from all (user-specific)
files in the list. This is because if credentials are only erased from
the file with the highest precedence, a matching credential may still be
found in a file further down the list. (Note that due to the lockfile
code, this requires the directory to be writable, which should be so for
user-specific config files)
store_credential() will write the credentials to the first existing
(user-specific) file in the list. If none of the files in the list
exist, store_credential() will write to the filename specified by the
first item of the filename list. For backwards compatibility, this
filename should be "~/.git-credentials".
Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This patch puts the usage info strings that were not already in docopt-
like format into docopt-like format, which will be a litle easier for
end users and a lot easier for translators. Changes include:
- Placing angle brackets around fill-in-the-blank parameters
- Putting dashes in multiword parameter names
- Adding spaces to [-f|--foobar] to make [-f | --foobar]
- Replacing <foobar>* with [<foobar>...]
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the interface declaration for the functions in lockfile.c from
cache.h to a new file, lockfile.h. Add #includes where necessary (and
remove some redundant includes of cache.h by files that already
include builtin.h).
Move the documentation of the lock_file state diagram from lockfile.c
to the new header file.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sparse issues 68 errors (two errors for each main() function) such
as the following:
SP git.c
git.c:510:5: error: too many arguments for function mingw_main
git.c:510:5: error: symbol 'mingw_main' redeclared with different type \
(originally declared at git.c:510) - different argument counts
The errors are caused by the 'main' macro used by the MinGW build
to provide a replacement main() function. The original main function
is effectively renamed to 'mingw_main' and is called from the new
main function. The replacement main is used to execute certain actions
common to all git programs on MinGW (e.g. ensure the standard I/O
streams are in binary mode).
In order to suppress the errors, we change the macro to include the
parameters in the declaration of the mingw_main function.
Unfortunately, this change provokes both sparse and gcc to complain
about 9 calls to mingw_main(), such as the following:
CC git.o
git.c: In function 'main':
git.c:510: warning: passing argument 2 of 'mingw_main' from \
incompatible pointer type
git.c:510: note: expected 'const char **' but argument is of \
type 'char **'
In order to suppress these warnings, since both of the main
functions need to be declared with the same prototype, we
change the declaration of the 9 main functions, thus:
int main(int argc, char **argv)
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is like "cache", except that we actually put the
credentials on disk. This can be terribly insecure, of
course, but we do what we can to protect them by filesystem
permissions, and we warn the user in the documentation.
This is not unlike using .netrc to store entries, but it's a
little more user-friendly. Instead of putting credentials in
place ahead of time, we transparently store them after
prompting the user for them once.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano
Carlo Marcelo Arenas Belón
Matthew DeVore
Nguyễn Thái Ngọc Duy
Jeff King
SZEDER Gábor
Michael Haggerty
Paul Tan
Alex Henrie
Ramsay Jones